Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol context and target.source-map setting
I've implemented original_file and ApplyFileMappings in LineEntry, as well as
changing some of the use of LineEntry.file to original_file, and change some
places that set file to also set original_file. I tried to have places that
cared about the symbol use original_file, but places that cared about the
source use the mapped file. This fixed my stepping problem. I'm building ToT
with this change, and will run tests on Linux. After they pass, I'll push a
patch to phabricator.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
-Original Message-
From: jing...@apple.com [mailto:jing...@apple.com]
Sent: Monday, May 09, 2016 1:06 PM
To: Ted Woodward
Cc: Greg Clayton ; LLDB
Subject: Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol context
and target.source-map setting
I talked with Greg about this before I posted the suggestion below, and having
the LineEntry contain both mapped and unmapped FileSpec's was his suggestion...
But who knows what changes of heart a weekend might bring.
Jim
> On May 9, 2016, at 9:43 AM, Ted Woodward wrote:
>
> I'll defer to Greg on remapping FileSpecs in SymbolContexts - he's the one
> who added it, in r168845:
>
> commit 214d2a327feb2e5987d093c2b63c457ffbd1d677
> Author: Greg Clayton
> Date: Thu Nov 29 00:53:06 2012 +
>
>
>
>Make stack frames fix up their line table entries when the target has
> source remappings. Also rearranged how the m_sc.target_sp was filled in so it
> can be used during the StackFrame::GetSymbolContext(...) function.
>
>
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?r1=168845&r2=168844&pathrev=168845
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
> -Original Message-
> From: jing...@apple.com [mailto:jing...@apple.com]
> Sent: Friday, May 06, 2016 6:01 PM
> To: Ted Woodward
> Cc: LLDB
> Subject: Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol context
> and target.source-map setting
>
> Another alternative would be to make the LineEntry hold both the remapped &
> original file spec, and add an API to get the "original FileSpec". Then
> anybody who is holding onto a line entry or file spec derived therefrom will
> know to compare the original file spec, since the remapped one is unreliable.
> Probably should move the remapping to the LineEntry
> (LineEntry::ApplyFileMappings(Target &target) so it is clear who is in charge
> of this.
>
> Note, you absolutely should never use SourceMaps to remap FileSpecs that are
> in the module's debug information, since these modules are shared among all
> the Debugger's, and the Debuggers could have different SourceMaps. So by
> putting this explicitly in the LineEntry - which is not something we use to
> store line information, it's just something we cons up to hand out when
> requested - it might make that requirement clearer.
>
> Jim
>
>
>> On May 6, 2016, at 3:49 PM, Jim Ingham via lldb-dev
>> wrote:
>>
>> Why are we remapping the FileSpecs in SymbolContext's we are handing out?
>> That seems to me a bad idea. It means that every time I want to do a
>> FileSpec compare between the LineEntry FileSpec's that I get at from stack
>> frames at two different times, I have to remember to re-apply the SourceMap
>> to the file spec I've stored away before doing the compare. After all, the
>> user might have changed the source map and thus the file spec we are handing
>> out. That seems very error prone. And we can't handle that in the FileSpec
>> "==" operator because FileSpec's don't have Targets, so they have no way of
>> getting to the map.
>>
>> Wouldn't it be better to use the source maps when we print filenames and
>> look for actual source files, and leave the symbol context FileSpec's in
>> some canonical form we know won't change over time?
>>
>> Jim
>>
>>
>>> On May 6, 2016, at 3:05 PM, Ted Woodward
>>> wrote:
>>>
>>> Symbols are being remapped. StackFrame::GetSymbolContext does this:
>>>
>>> m_sc.line_entry = sc.line_entry;
>>> if (m_sc.target_sp)
>>> {
>>> // Be sure to apply and file remappings to our file
>>> and line
>>> // entries when handing out a line entry
>>> FileSpec new_file_spec;
>>> if (m_sc.target_sp->GetSourcePathMap().FindFile
>>> (m_sc.line_entry.file, new_file_spec))
>>> m_sc.line_entry.file = new_file_spec;
>>> }
>>>
>>> This code gets called if the StackFrame ctor is called with the
>>> SymbolContext = nullptr, but this is skipped if the SymbolContext is valid.
>>> All new StackFrames in StackFrameList are done with a null SC, except for
>>>
Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol context and target.source-map setting
Excellent, thanks!
Jim
> On May 10, 2016, at 10:51 AM, Ted Woodward
> wrote:
>
> I've implemented original_file and ApplyFileMappings in LineEntry, as well as
> changing some of the use of LineEntry.file to original_file, and change some
> places that set file to also set original_file. I tried to have places that
> cared about the symbol use original_file, but places that cared about the
> source use the mapped file. This fixed my stepping problem. I'm building ToT
> with this change, and will run tests on Linux. After they pass, I'll push a
> patch to phabricator.
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
> -Original Message-
> From: jing...@apple.com [mailto:jing...@apple.com]
> Sent: Monday, May 09, 2016 1:06 PM
> To: Ted Woodward
> Cc: Greg Clayton ; LLDB
> Subject: Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol context
> and target.source-map setting
>
> I talked with Greg about this before I posted the suggestion below, and
> having the LineEntry contain both mapped and unmapped FileSpec's was his
> suggestion... But who knows what changes of heart a weekend might bring.
>
> Jim
>
>> On May 9, 2016, at 9:43 AM, Ted Woodward wrote:
>>
>> I'll defer to Greg on remapping FileSpecs in SymbolContexts - he's the one
>> who added it, in r168845:
>>
>> commit 214d2a327feb2e5987d093c2b63c457ffbd1d677
>> Author: Greg Clayton
>> Date: Thu Nov 29 00:53:06 2012 +
>>
>>
>>
>> Make stack frames fix up their line table entries when the target has
>> source remappings. Also rearranged how the m_sc.target_sp was filled in so
>> it can be used during the StackFrame::GetSymbolContext(...) function.
>>
>>
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?r1=168845&r2=168844&pathrev=168845
>>
>> --
>> Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>> Linux Foundation Collaborative Project
>>
>>
>> -Original Message-
>> From: jing...@apple.com [mailto:jing...@apple.com]
>> Sent: Friday, May 06, 2016 6:01 PM
>> To: Ted Woodward
>> Cc: LLDB
>> Subject: Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol
>> context and target.source-map setting
>>
>> Another alternative would be to make the LineEntry hold both the remapped &
>> original file spec, and add an API to get the "original FileSpec". Then
>> anybody who is holding onto a line entry or file spec derived therefrom will
>> know to compare the original file spec, since the remapped one is
>> unreliable. Probably should move the remapping to the LineEntry
>> (LineEntry::ApplyFileMappings(Target &target) so it is clear who is in
>> charge of this.
>>
>> Note, you absolutely should never use SourceMaps to remap FileSpecs that are
>> in the module's debug information, since these modules are shared among all
>> the Debugger's, and the Debuggers could have different SourceMaps. So by
>> putting this explicitly in the LineEntry - which is not something we use to
>> store line information, it's just something we cons up to hand out when
>> requested - it might make that requirement clearer.
>>
>> Jim
>>
>>
>>> On May 6, 2016, at 3:49 PM, Jim Ingham via lldb-dev
>>> wrote:
>>>
>>> Why are we remapping the FileSpecs in SymbolContext's we are handing out?
>>> That seems to me a bad idea. It means that every time I want to do a
>>> FileSpec compare between the LineEntry FileSpec's that I get at from stack
>>> frames at two different times, I have to remember to re-apply the SourceMap
>>> to the file spec I've stored away before doing the compare. After all, the
>>> user might have changed the source map and thus the file spec we are
>>> handing out. That seems very error prone. And we can't handle that in the
>>> FileSpec "==" operator because FileSpec's don't have Targets, so they have
>>> no way of getting to the map.
>>>
>>> Wouldn't it be better to use the source maps when we print filenames and
>>> look for actual source files, and leave the symbol context FileSpec's in
>>> some canonical form we know won't change over time?
>>>
>>> Jim
>>>
>>>
On May 6, 2016, at 3:05 PM, Ted Woodward
wrote:
Symbols are being remapped. StackFrame::GetSymbolContext does this:
m_sc.line_entry = sc.line_entry;
if (m_sc.target_sp)
{
// Be sure to apply and file remappings to our file
and line
// entries when handing out a line entry
FileSpec new_file_spec;
if (m_sc.target_sp->GetSourcePathMap().FindFile
(m_sc.line_entry.file, new_file_spec))
m_sc.line_entry.file = new_file_spec;
}
This code g
