[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

2018-09-25 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In https://reviews.llvm.org/D48393#1245049, @clayborg wrote: > In https://reviews.llvm.org/D48393#1244649, @labath wrote: > > > I agree with Greg that it would be best to restrict things such that there > > is only one instance of parsing going on at any given time for a s

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. I do agree it's slightly easier to read this way. I was conflicted while writing the tests between readability and conciseness. I think this is a good compromise. I do prefer to have the check lines with the commands rather than with the source code though. The output is

[Lldb-commits] [PATCH] D55319: [CMake] Proposal: Prefer LLDB_VERSION over plist value in EmbedAppleVersion.cmake

2018-12-05 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. Wouldn't it make even more sense to to inject LLDB_VERSION into the Info.plist? We will use the Info.plist afterwards, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55319/new/ https://reviews.llvm.org/D55319 _

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

2018-12-05 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D55328#1320267 , @clayborg wrote: > I like seeing all of the cmake modifications for the LLDB.framework. Are we > planning on trying to get rid of the Xcode project at some point soon and use > the auto generated one made by cma

[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

2018-12-13 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. Zachary, how did you figure out this can be an issue? Does it fix something we should be testing? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55571/new/ https://reviews.llvm.org/D55571 ___

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2018-12-19 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. > Unfortunately, I don't think llvm has anything like that, though I think it > would be extremely useful (/me looks at apple folks). If you try hard enough, > you should be able to get clang to produce a dsym bundle for you even on > linux. This did the trick for me: >

[Lldb-commits] [PATCH] D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX)

2018-12-19 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Host/common/Symbols.cpp:365 + if (!external_lookup) { +Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); jankratochvil wrote: > labath wrote: > > This doesn't sound right to me. It looks li

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

2018-05-10 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: jingham. Herald added a subscriber: emaste. Multiple threads could be calling into DoLoadImage concurrently, only one should be allowed to create the UtilityFunction. https://reviews.llvm.org/D46733 Files: source/Plugins/Platform/POSIX/Plat

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

2018-05-10 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1046-1047 + { +static std::mutex do_dlopen_mutex; +std::lock_guard lock(do_dlopen_mutex); + clayborg wrote: > We should put the mutex, or better yet a std::once_flag

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

2018-05-10 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 146268. friss added a comment. I heted the idea of hosting the once_flag in Process but using it in PlatformPOSIX. I did a bigger refactoring where platforms pass a factory lambda to the accessor and the thread-safe init is done in the Process class itself using

[Lldb-commits] [PATCH] D46736: HostInfoMacOSX: Share the clang resource directory with Swift.

2018-05-11 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision. friss added inline comments. This revision is now accepted and ready to land. Comment at: source/Host/macosx/HostInfoMacOSX.mm:288 + // Fall back to the Clang resource directory inside the framework. + raw_path.resize(rev_it - r_end); raw_path.a

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

2018-05-11 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL332115: Add a lock to PlatformPOSIX::DoLoadImage (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46733?vs=146268&id=146368

[Lldb-commits] [PATCH] D48303: Don't take the address of an xvalue when printing an expr result

2018-06-19 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/xvalue/TestXValuePrinting.py:32 +self.assertTrue(value.GetError().Success()) +self.assertEqual(value.GetValueAsUnsigned(), 0) + Could you use a different v

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

2018-06-20 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: clayborg, labath, jingham. Herald added subscribers: JDevlieghere, aprantl. Debug information is read lazily so a lot of the state of the reader is constructred incrementally. There's also nothing preventing symbol queries to be performed at the

[Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

2018-06-20 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In https://reviews.llvm.org/D48393#1138398, @zturner wrote: > Long term I think one potentially interesting possibility for solving a lot > of these threading and lazy evaluation issues is to make a task queue that > runs all related operations on a single thread and retu

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-20 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. Stefan, you should always add lldb-commits to the subscribers of your phab reviews. I added it now, but IIRC there are issues with adding subscribers after the fact. https://reviews.llvm.org/D49612 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D50536: Fix: ConstString::GetConstCStringAndSetMangledCounterPart() should update the value if the key exists already

2018-08-10 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Utility/ConstString.cpp:123-126 + assert((map.find(demangled) == map.end() || strlen(map[demangled]) == 0 || + map[demangled] == mangled_ccstr) && + "The demangled string must have a unique counterpart

[Lldb-commits] [PATCH] D51245: Allow IRInterpreter to deal with non-power-of-2 sized types to support some bitfield accesses.

2018-08-24 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jingham, clayborg. For some bitfield patterns (like the one added by this commit), Clang will generate non-regular data types like i24 or i48. This patch follows a pretty naive approach of just bumping the type size to the next power of 2. DataEx

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-28 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jingham, clayborg. Herald added a subscriber: JDevlieghere. When looking at template arguments in LLDB, we usually care about what the user passed in his code, not whether some of those arguments where passed as a variadic parameter pack. This p

[Lldb-commits] [PATCH] D51245: Allow IRInterpreter to deal with non-power-of-2 sized types to support some bitfield accesses.

2018-08-28 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340880: Allow IRInterpreter to deal with non-power-of-2 sized types to support some… (authored by friss, committed by ). Herald added a subscriber: abidh. Changed prior to commit: https://reviews.llv

[Lldb-commits] [PATCH] D51245: Allow IRInterpreter to deal with non-power-of-2 sized types to support some bitfield accesses.

2018-08-28 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340880: Allow IRInterpreter to deal with non-power-of-2 sized types to support some… (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llv

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-29 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } shafik wrote: > Is this saying that an empty parameter pa

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-30 Thread Frederic Riss via Phabricator via lldb-commits
friss marked 2 inline comments as done. friss added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } friss wrote: > sh

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-30 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Symbol/ClangASTContext.cpp:7664 + + assert(args.size() && + "We shouldn't have a template specialization without any args"); friss wrote: > shafik wrote: > > The asset before we do math making this assumpti

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-08-30 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 163399. friss added a comment. Address feedback https://reviews.llvm.org/D51387 Files: include/lldb/Symbol/ClangASTContext.h include/lldb/Symbol/CompilerType.h include/lldb/Symbol/TypeSystem.h packages/Python/lldbsuite/test/lang/cpp/class-template-pa

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

2018-09-13 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. @jingham ping https://reviews.llvm.org/D51387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43546: Fix TestMultithreaded when specifying an alternative debugserver.

2018-02-20 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. This test launches a helper that uses the debugserver. The environment variable sepcifying the debug server wasn't passed to this helper, thus it was using the default one. I'd love to hear if anyone has a nicer idea how to get access to the alternative server passed

[Lldb-commits] [PATCH] D43577: Fix TestUbsanBasic

2018-02-21 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: vsk. Herald added a subscriber: kubamracek. Potentially due to the recent testuite refactorings, this test now reports a full absolute path but expect just the filename. For some reason this test is skipped on GreenDragon so we've never seen the

[Lldb-commits] [PATCH] D43592: [DWARFASTParserClang] Always complete types read from a module/PCH AST context.

2018-02-21 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: clayborg. Herald added subscribers: JDevlieghere, aprantl. The modified test would just crash without the code change. The reason is that we would try to extend the Foo type imported from the PCH debug info when adding the Foo::Bar definitiion

[Lldb-commits] [PATCH] D43592: [DWARFASTParserClang] Always complete types read from a module/PCH AST context.

2018-02-22 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. Note that this code path is only triggered when importing debug info from a different AST context, it is not the common codepath. The issue in this case is that LLDB is crashing when using the incomplete Decl as the DeclContext for another one. I guess I could add calls t

[Lldb-commits] [PATCH] D43546: Fix TestMultithreaded when specifying an alternative debugserver.

2018-02-22 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325858: Fix TestMultithreaded when specifying an alternative debugserver. (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D4354

[Lldb-commits] [PATCH] D43577: Fix TestUbsanBasic

2018-02-22 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325859: Fix TestUbsanBasic (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43577?vs=135263&id=135590#toc Repository: rL

[Lldb-commits] [PATCH] D43592: [DWARFASTParserClang] Always complete types read from a module/PCH AST context.

2018-03-16 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 138733. friss added a comment. I experimented quite a bit with different approches to fix this bug without always completing the types right after importing them from the external AST. This is the most principled approach I came up with. I applied the new helpe

[Lldb-commits] [PATCH] D43592: [DWARFASTParserClang] Always complete types read from a module/PCH AST context.

2018-03-16 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 138734. friss added a comment. Adding back a hunk I didn't mean to delete. https://reviews.llvm.org/D43592 Files: packages/Python/lldbsuite/test/lang/cpp/gmodules/TestWithModuleDebugging.py packages/Python/lldbsuite/test/lang/cpp/gmodules/main.cpp packa

[Lldb-commits] [PATCH] D43592: [DWARFASTParserClang] Always complete types read from a module/PCH AST context.

2018-03-16 Thread Frederic Riss via Phabricator via lldb-commits
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 rL327750: [DWARFASTParserClang] Complete external record types before using them as a… (authored by friss, committed by ). H

[Lldb-commits] [PATCH] D44613: Support template template parameters

2018-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: clayborg, jingham. Herald added a subscriber: JDevlieghere. We would fail to resolve (and thus display the value of) any templated type which contained a template template argument even though we don't really use template arguments. This patch a

[Lldb-commits] [PATCH] D44613: Support template template parameters

2018-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2043 +is_template_template_argument = true; +[[clang::fallthrough]]; case DW_TAG_template_type_parameter: clayborg wrote: > Shouldn't we be using a #define

[Lldb-commits] [PATCH] D44613: Support template template parameters

2018-03-23 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 139619. friss added a comment. Use LLVM macro for fallthrough https://reviews.llvm.org/D44613 Files: include/lldb/Symbol/ClangASTContext.h packages/Python/lldbsuite/test/lang/cpp/template/TestTemplateArgs.py packages/Python/lldbsuite/test/lang/cpp/templ

[Lldb-commits] [PATCH] D45011: Prevent double release of mach ports

2018-03-28 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: jasonmolenda. When a MIG routine returns KERN_FAILURE, the demux function will release any OOL resources like ports. In this case, task_port and thread_port will be released twice, potentially resulting in use after free of the ports. I don't

[Lldb-commits] [PATCH] D45011: Prevent double release of mach ports

2018-03-28 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL328761: Prevent double release of mach ports (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D45011 Files: lldb/trunk/source

[Lldb-commits] [PATCH] D44613: Support template template parameters

2018-04-01 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. Ping https://reviews.llvm.org/D44613 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44613: Support template template parameters

2018-04-02 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL328984: Support template template parameters (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44613 Files: lldb/trunk/includ

[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

2018-04-04 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jasonmolenda, labath. In one of the 2 places the LC_BUILD_VERSION load command is handled, there is a bug preventing us from actually handling them (the address where to read the load command was not updated). This patch factors reading the deplo

[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

2018-04-05 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py:16-18 +sim_devices_str = subprocess.check_output(['xcrun', 'simctl', 'list', '-j', + 'devices']) +sim_devic

[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

2018-04-05 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329374: [debugserver] Fix LC_BUILD_VERSION load command handling. (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45298?vs

[Lldb-commits] [PATCH] D46548: Really test type lookup in TestCppTypeLookup.py

2018-05-07 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: clayborg, jingham. ... and fix one bug found this way. Currently, the test works not because types are looked up correctly, but because by injecting local variables we also materialize the types for Clang. If we disable the local variable injecti

[Lldb-commits] [PATCH] D46551: Inject only relevant local variables in the expression evaluation context

2018-05-07 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jingham, clayborg. In r259902, LLDB started injecting all the locals in every expression evaluation. This fixed a bunch of issues, but also caused others, mostly performance regressions on some codebases. The regressions were bad enough that we a

[Lldb-commits] [PATCH] D46548: Really test type lookup in TestCppTypeLookup.py

2018-05-07 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL331719: Really test type lookup in TestCppTypeLookup.py (authored by friss, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46548 Files: lldb/

[Lldb-commits] [PATCH] D58219: [dotest] Fix compiler version number comparison

2019-02-13 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: zturner, labath. Herald added a reviewer: serge-sans-paille. Herald added a subscriber: jdoerfert. dotest's version comparision function is just a lexicographical compare of the version strings. This is obviously wrong. This patch implements a nu

[Lldb-commits] [PATCH] D58219: [dotest] Fix compiler version number comparison

2019-02-13 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 186782. friss added a comment. Use LooseVersion as suggested by Zachary CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58219/new/ https://reviews.llvm.org/D58219 Files: packages/Python/lldbsuite/test/lldbtest.py Index: packages/Python/lldbsuite/te

[Lldb-commits] [PATCH] D58219: [dotest] Fix compiler version number comparison

2019-02-14 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL354047: [dotest] Fix compiler version number comparison (authored by friss, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[Lldb-commits] [PATCH] D58912: [debugserver] Fix IsUserReady thread filtering

2019-03-04 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jasonmolenda, clayborg, jingham. Herald added a subscriber: jdoerfert. In 2010 (r118866), filtering code was added to debugserver to avoid reporting threads that were "not ready to be displayed to the user". This code inspects the thread's stat

[Lldb-commits] [PATCH] D58912: [debugserver] Fix IsUserReady thread filtering

2019-03-04 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D58912#1417570 , @jasonmolenda wrote: > Will this hide a thread that jumps through a null function pointer? That's > the only user process case where a pc of 0 needs to be reported to the > developer. Nope, I tested this exp

[Lldb-commits] [PATCH] D58912: [debugserver] Fix IsUserReady thread filtering

2019-03-06 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB35: [debugserver] Fix IsUserReady thread filtering (authored by friss, committed by ). Herald added a subscriber: abidh. Herald added a project: LLDB. Changed prior to commit: https://reviews.llv

[Lldb-commits] [PATCH] D59847: Regression test to ensure that we handling importing of std::vector of enums correctly

2019-03-26 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. As we're just adding test coverage, could we add a little more? - Anonymous enum - Enum through a typedef - class enum - enum declared inside of the function rather than at the top-level - nested enum in a record type - enum nested in a templated class - anything else I hav

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

2019-03-28 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:171 if (!var_name || var_name == ConstString("this") || +var_name == ConstString("self") || var_name == ConstString("_cmd") || var_name == ConstString(".block_descriptor")) -

[Lldb-commits] [PATCH] D60178: [Reproducers] Capture return values of functions returning by ptr/ref

2019-04-03 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision. friss added a comment. This revision is now accepted and ready to land. This seems super mechanical and we discussed it at length offline. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60178/new/ https://reviews.llvm.org/D60178 _

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

2019-04-09 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: clayborg. Before a Debugger gets a Target, target settings are routed to a global set of settings. Even without this, some part of the LLDB which exist independently of the Debugger object (the Module cache, the Symbol vendors, ...) access direc

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

2019-04-09 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D60468#1460013 , @clayborg wrote: > Almost seems like we can build the mutex into the base class OptionValue as > we need general threaded protection for every setting. They any function that > gets or sets the value should be a

[Lldb-commits] [PATCH] D60468: Lock accesses to OptionValueFileSpecList objects

2019-04-09 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D60468#1460065 , @labath wrote: > No opinion on the patch, but what is the reason for having settings that are > shared between multiple Debugger instances? My expectation was that the > debugger objects are completely independe

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

2019-04-09 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: source/Expression/ExpressionSourceCode.cpp:181 + +if (var_name == ConstString("this") && wrapping_language == lldb::eLanguageTypeC_plus_plus ) continue; This should also trigger for eLanguageTypeObjC_plus_plus

[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

2019-04-10 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. You didn't address my comment that "this" needs to treated specially in Obj-C++ too. Other than that this LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59960/new/ https://reviews.llvm.org/D59960 ___ lldb-commits

[Lldb-commits] [PATCH] D60862: [CMake] Allow custom extensions for externalized debug info

2019-04-18 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision. friss 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/D60862/new/ https://reviews.llvm.org/D60862 ___ l

[Lldb-commits] [PATCH] D60862: [CMake] Allow custom extensions for externalized debug info

2019-04-18 Thread Frederic Riss via Phabricator via lldb-commits
friss requested changes to this revision. friss added a comment. This revision now requires changes to proceed. Actually, thinking more about this, how do you use it? LLVM_EXTERNALIZE_DEBUGINFO_EXTENSION is a global but this would be different per target. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D60862: [CMake] Allow custom extensions for externalized debug info

2019-04-18 Thread Frederic Riss via Phabricator via lldb-commits
friss accepted this revision. friss added a comment. This revision is now accepted and ready to land. Interesting. Cmake scoping rules are still a mystery to me. So when you `set(FOO ...), it's set only for the current scope and in any functions/macros you call in that scope? Then this seems lik

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

2022-11-16 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h:81-82 + +/// Is DIE a forward or backward reference? +bool Reference : 1; }; When I read the comment, I was confused as I thought the flag was used to diff

[Lldb-commits] [PATCH] D74187: [lldb] Add method Language::IsMangledName

2020-02-07 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:96-98 + bool IsMangledName(llvm::StringRef name) const override { +return false; + } shafik wrote: > xiaobai wrote: > > friss wrote: > > > The original code was callin

[Lldb-commits] [PATCH] D74157: [lldb/API] NFC: Reformat SBThread::GetStopDescription()

2020-03-02 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG20ce8affce85: [lldb/API] NFC: Reformat and simplify SBThread::GetStopDescription() (authored by friss). Changed prior to commit: https://reviews.llvm.org/D74157?vs=243239&id=247770#toc Repository: rG

[Lldb-commits] [PATCH] D75696: Correctly detect iOS simulator processes.

2020-03-05 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:601-607 +static bool IsMacOSHost() { + static uint32_t g_host_cputype = 0; + size_t len = sizeof(uint32_t); + if (!g_host_cputype) +::sysctlbyname("hw.cputype", &g_host_cputype, &l

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

2020-03-11 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jingham, labath. Herald added a project: LLDB. The TargetProperties constructor invokes a series of callbacks to prime the properties from the default ones. The one callback in charge of updating the inferior environment was commented out because

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

2020-03-11 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jingham, labath. Herald added projects: LLDB, libc++abi. Herald added a subscriber: libcxx-commits. Herald added a reviewer: libc++abi. friss updated this revision to Diff 249834. friss added a comment. friss removed a reviewer: libc++abi. friss r

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

2020-03-11 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 249834. friss added a comment. Remove unrelated change commited by accident Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76045/new/ https://reviews.llvm.org/D76045 Files: lldb/include/lldb/API/SBTarget.h ll

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

2020-03-12 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D76009#1919103 , @labath wrote: > Actually, hang on. > > > One existing test had to be modified, because the initialization of > > the environment properties now take place at the time the target is > > created, not at the first

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

2020-03-12 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. I put up a "fix" for the inherit-env issue mentioned here: https://reviews.llvm.org/D76105 It is mostly orthogonal to this patch as Jim showed the behavior today is already broken. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

2020-03-12 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: labath, jingham. Herald added a project: LLDB. Allow the target.env-vars property to be recomputed when the target.inherit-env property changes. This allows to change the value of the property between runs and get a meaningful behavior. Reposit

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

2020-03-12 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. This is somewhat useless without D76009 because the default LaunchInfo doesn't get populated with the environment without it. I'll wait for a resolution there before landing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D76105: [lldb/settings] Reset the inferior environment when target.inherit-env is toggled

2020-03-12 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D76105#1920633 , @jingham wrote: > If I'm following the logic correctly, if you run a target with inherit-env > off, and then do: > > env VAR_IN_ENVIRONMENT=NOT_THE_ENVIRONMENTS_VALUE > > > > then turn inherit-env on, we will

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

2020-03-14 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: lldb/include/lldb/Target/StackFrameRecognizer.h:115 std::function const - &callback); + std::string module, std::vector &symbols, + bool regexp)> const &callback); ---

[Lldb-commits] [PATCH] D76188: [lldb/Target] Support more than 2 symbols in StackFrameRecognizer

2020-03-15 Thread Frederic Riss via Phabricator via lldb-commits
friss added inline comments. Comment at: lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py:162-167 +lldbutil.run_break_set_by_symbol(self, "bar") +self.runCmd("c") + +process = target.GetProcess() +thread = process.GetSelectedThread()

[Lldb-commits] [PATCH] D76341: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

2020-03-17 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: jasonmolenda, kubamracek. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: LLDB. The memory history plugin for Asan creates a HistoryThread with the recorded PC values provided by the Asan runtime. In other cases, thos

[Lldb-commits] [PATCH] D76341: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb40ee7ff1b16: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces (authored by friss). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7634

[Lldb-commits] [PATCH] D76406: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: labath, jingham. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: LLDB. TestInlineStepping tests LLDB's ability to step in the presence of inline frames. The testcase source has a number of functions and some of them a

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: mib, jingham. Herald added a project: LLDB. The crash info annotations are not necessarily indicative of a crash. Some APIs will set them before doing something risk and clean them up after, while some others might just leave a breadcrumb there f

[Lldb-commits] [PATCH] D76408: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

2020-03-18 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: labath. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: LLDB. TestBuiltinTrap fail on darwin embedded because the `__builin_trap` builtin doesn't get any line info attached to it by clang when building for arm64. Th

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss marked an inline comment as done. friss added a comment. In D76407#1930693 , @labath wrote: > Could the inferior manually wipe the crash annotation? This is not something you're supposed to wipe. It's additional information that is going to b dump

[Lldb-commits] [PATCH] D76407: [lldb/testsuite] Remove "healthy process" test from TestProcessCrashInfo.py

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss abandoned this revision. friss added a comment. I skipped the test for embedded in 8758d02074be7b80b804fad19e8b7de6ebd43c31 . I'll abandon this for now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D76406: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGecc6c426977f: [lldb/testsuite] Fix TestInlineStepping on arm64 with newer compilers (authored by friss). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76406/

[Lldb-commits] [PATCH] D76408: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe154cbb124a6: [lldb/testsuite] XFail TestBuiltinTrap.py not only on linux (authored by friss). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76408/new/ http

[Lldb-commits] [PATCH] D76450: [lldb/PlatformDarwin] Always delete destination file first in PutFile

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added a reviewer: jasonmolenda. Herald added a project: LLDB. The default behavior of Platform::PutFile is to open the file and truncate it if it already exists. This works fine and is a sensible default, but it interacts badly with code-signing on iOS, as doing

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss created this revision. friss added reviewers: labath, jingham. Herald added a project: LLDB. friss added a comment. This needs some tests, but I wanted to put it out there for comments. The interactions between the environment settings (`target.env-vars`, `target.inherit-env`) and the infe

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-19 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. This needs some tests, but I wanted to put it out there for comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76470/new/ https://reviews.llvm.org/D76470 ___ lldb-commits ma

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D76470#1933062 , @labath wrote: > I think this is a good change, and is in line with what we have discussed > previously. I have just one question about the exact variable interactions > here. > > Since the raison d'être of the

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 251670. friss added a comment. Add tests and update m_launch_info when undet-vars change. Improve docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76470/new/ https://reviews.llvm.org/D76470 Files: lldb/incl

[Lldb-commits] [PATCH] D76009: [lldb/Target] Initialize new targets environment variables from target.env-vars

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 251675. friss added a comment. Make helper function name more descriptive. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76009/new/ https://reviews.llvm.org/D76009 Files: lldb/include/lldb/Target/Target.h ll

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D76470#1933910 , @jingham wrote: > Is there any command-based way to see the entire environment that a process > will get when it launches? By populating target.env-vars with the inherited > environment there was a way to mostl

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 251794. friss added a comment. Herald added a subscriber: mgrang. Add a new target command to dump the launch environment Make env-vars override unset-env-vars Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76470/n

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. In D76470#1934201 , @jingham wrote: > In D76470#1934146 , @friss wrote: > > > In D76470#1933910 , @jingham wrote: > > > > > Is there any command-based

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-20 Thread Frederic Riss via Phabricator via lldb-commits
friss updated this revision to Diff 251813. friss added a comment. Simplify command definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76470/new/ https://reviews.llvm.org/D76470 Files: lldb/include/lldb/Target/Target.h lldb/source/Comman

[Lldb-commits] [PATCH] D76470: [lldb/Target] Rework the way the inferior environment is created

2020-03-23 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb4a6e63ea123: [lldb/Target] Rework the way the inferior environment is created (authored by friss). Changed prior to commit: https://reviews.llvm.org/D76470?vs=251813&id=252052#toc Repository: rG LLV

[Lldb-commits] [PATCH] D76045: [lldb/API] Make Launch(Simple) use args and env from target properties

2020-03-23 Thread Frederic Riss via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGcd7b45057ca9: [lldb/API] Make Launch(Simple) use args and env from target properties (authored by friss). Changed prior to commit: https://reviews.llvm.org/D76045?vs=249834&id=252051#toc Repository:

  1   2   3   >