jarin added a comment. In D110571#3033078 <https://reviews.llvm.org/D110571#3033078>, @labath wrote:
> Here's one more question. AIUI, lldb relies on the order of formal parameter > declarations in dwarf to establish the the function signature (dwarf doesn't > leave us much choice. This then affects how the function is printed in the > backtrace, for instance. What will be the resulting order of arguments for > these functions? I'm wondering if we don't need a two-pass algorithm, which > first parses the arguments in the function declaration (to establish their > order), and then do another pass over the concrete instance to fill in the > missing information. (I'm sorry if you're doing this already, but I'm still > too scared of the code to figure it out myself :P ). The code already does the merging. For DW_TAG_inlined_subroutine, it first collects the formal_parameter list from from its abstract_origin and then it will parse/insert all the missing one while parsing the concrete instance. This will preserve the order of formal parameters. Now that I think about this, it might add some formal parameters after local variables, but I hope this is not a real problem for LLDB. If this is a problem, we could perhaps flush the abstract formal parameters whenever we encounter DW_TAG_variable. > In D110571#3032599 <https://reviews.llvm.org/D110571#3032599>, @jarin wrote: > >> From what you say, this is not the desired behavior? If we wanted two >> instances of the variable (one for each block), we could change the >> DIE-to-variable cache >> <https://github.com/llvm/llvm-project/blob/47d66355ef9039a15b7265945e3deb331d7f9e05/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h#L324> >> to be indexed by a pair <symbol-context-DIE, variable-DIE>. > > Given that was Greg's (and yours, kinda) reaction as well. I guess we should > do something like that. Even if it does not cause problems now, it could > certainly cause them in the future, if something starts relying on the > symbol_context_scope link making sense. > > I am wondering about the best way to implement in though. Having a pair as a > key seems very redundant to me. As we already know the block its going to end > up in maybe we could somehow check if its already present there? Since > blocks/functions don't generally have that many variables [citation needed], > maybe even a simple iteration would suffice? (The situation is probably > different for global variables, but those don't need the extra key.) Are you worried about code redundancy or memory redundancy? I do not think a pair would be much extra code. If you are worried about memory, we could also have a separate map for the abstract parameters - we always know whether we are inserting an abstract parameter or a concrete one. (I did not quite understand the idea with block lookup/iteration.) An interesting question is whether the caching is needed at all in the context of functions - even without the cache, we should not parse block variables multiple times because the variables are already cached in their block's variable list. I actually verified that the cache never hits for function scoped variables on the LLDB test suite (with and without this patch). It does hit for global variables, but they take a different path now. So how would you feel about bypassing the cache when parsing in the function context? (I would basically move the caching code from SymbolFileDWARF::ParseVariableDIE to SymbolFileDWARF::ParseAndAppendGlobalVariable.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
