[Lldb-commits] [PATCH] D17860: Fix "ninja check-lldb" crash in IRExecutionUnit.cpp

[Jim Ingham via lldb-commits]

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 <ted.woodward at codeaurora.org> 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: jingham at apple.com [mailto:jingham at apple.com] 
> Sent: Tuesday, March 08, 2016 5:35 PM
> To: reviews+D17860+public+90c8568c5fdb4800 at reviews.llvm.org
> Cc: ted.woodward at codeaurora.org; zturner at google.com; scallanan at apple.com;
> amccarth at google.com; lldb-commits at lists.llvm.org
> Subject: Re: [PATCH] D17860: Fix "ninja check-lldb" crash in
> IRExecutionUnit.cpp
> 
> 
>> On Mar 8, 2016, at 2:46 PM, Ted Woodward <ted.woodward at codeaurora.org>
> 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
>> 
>> 
>> 
> 
>