Right. If we have both, the symbol should be faster and won't potentially drag in more debug info, so we should use it. If the two have different addresses that's a more fundamental problem we shouldn't try to patch over here. We should only use the function if it's all we have. Note also, if there is a function but no symbol then it can't be an external symbol...
Jim > On Mar 9, 2016, at 11:37 AM, Ted Woodward <[email protected]> wrote: > > So you'd like to see this function get the address of a function that it > finds in either symbols or debug info? > > Which should it prioritize when we have both? > > Ted > -- > Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project > > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: Tuesday, March 08, 2016 5:35 PM > To: [email protected] > Cc: [email protected]; [email protected]; [email protected]; > [email protected]; [email protected] > Subject: Re: [PATCH] D17860: Fix "ninja check-lldb" crash in > IRExecutionUnit.cpp > > >> On Mar 8, 2016, at 2:46 PM, Ted Woodward <[email protected]> > wrote: >> >> ted added a comment. >> >> The change is to guard against the case where candidate_sc.symbol is > nullptr. >> >> candidate_sc.function is only used when load_address != > LLDB_INVALID_ADDRESS, but load_address is set on line 802: >> >> load_address = candidate_sc.symbol->ResolveCallableAddress(*target); >> >> so candidate_sc.symbol must be valid. >> >> The purpose of the function is to get the address of a symbol, so I don't > think we care about candidate_sc.function when candidate_sc.symbol is > nullptr. > > It's name is "FindInSymbols" but I am pretty sure that's in > contradistinction to "FindInRuntimes" not "FindInDebugInformation". The > searches that feed this function search both Symbols and Debug Information. > I agree with you that the original code worked incorrectly in the case where > you had a function from debug information and not from symbols, but your > change would need to be reverted to make this work properly (and states an > intent that I don't think is correct.) > > Jim > > >> >> >> http://reviews.llvm.org/D17860 >> >> >> > > _______________________________________________ lldb-commits mailing list [email protected] http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
