Excellent, thanks! Jim
> On May 10, 2016, at 10:51 AM, Ted Woodward <ted.woodw...@codeaurora.org> > 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 <ted.woodw...@codeaurora.org> > Cc: Greg Clayton <clayb...@gmail.com>; LLDB <lldb-dev@lists.llvm.org> > 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 <ted.woodw...@codeaurora.org> 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 <gclay...@apple.com> >> Date: Thu Nov 29 00:53:06 2012 +0000 >> >> <rdar://problem/12445557> >> >> 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 <ted.woodw...@codeaurora.org> >> Cc: LLDB <lldb-dev@lists.llvm.org> >> 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 >>> <lldb-dev@lists.llvm.org> 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 <ted.woodw...@codeaurora.org> >>>> 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 an inlined frame. In that case, StackFrameList::GetFramesUpTo >>>> calls SymbolContext::GetParentOfInlinedScope, which sets the SC, and >>>> GetFramesUpTo does not remap it like StackFrame::GetSymbolContext does. >>>> Then it creates a new StackFrame with the SC. >>>> >>>> Adding this before the new StackFrame fixes the issue: >>>> if (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 >>>> (target_sp->GetSourcePathMap().FindFile(next_frame_sc.line_entry.file, >>>> new_file_spec)) >>>> next_frame_sc.line_entry.file = new_file_spec; >>>> } >>>> >>>> I've put up a patch on Phabricator with Jim as reviewer. >>>> >>>> -- >>>> 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 2:41 PM >>>> To: Ted Woodward <ted.woodw...@codeaurora.org> >>>> Cc: LLDB <lldb-dev@lists.llvm.org> >>>> Subject: Re: [lldb-dev] Bug with ThreadPlanStepRange::InRange, symbol >>>> context and target.source-map setting >>>> >>>> >>>>> On May 6, 2016, at 11:22 AM, Ted Woodward via lldb-dev >>>>> <lldb-dev@lists.llvm.org> wrote: >>>>> >>>>> I’m stepping over the first line of a libcxx test (source >>>>> https://llvm.org/svn/llvm-project/libcxx/trunk/test/std/thread/thread.condition/thread.condition.condvar/wait.pass.cpp >>>>> ). The first line has an inlined function call. I expect lldb to step >>>>> over the breakpoint, run to the end of the range that sets up the inlined >>>>> function call, run through the inlined function call, then run to the end >>>>> of the line. Instead, it runs to the inlined call, then stops. >>>>> >>>>> I’m running lldb on Windows, debugging a Hexagon application that was >>>>> built on Linux. I’m using the target.source-map setting to let me see >>>>> source. >>>>> >>>>> The problem is in ThreadPlanStepRange::InRange. It checks to see if we’re >>>>> still on the same line by comparing the filename in the Stepping Plan’s >>>>> line entry to the filename in the current frame’s line entry. >>>>> m_addr_context.line_entry.file has been normalized by the value in >>>>> target.source-map, but new_context.line_entry.file hasn’t, so they’re not >>>>> the same, even though they should be. >>>>> >>>>> SymbolContext >>>>> new_context(frame->GetSymbolContext(eSymbolContextEverything)); >>>>> if (m_addr_context.line_entry.IsValid() && >>>>> new_context.line_entry.IsValid()) >>>>> { >>>>> if (m_addr_context.line_entry.file == new_context.line_entry.file) >>>>> { >>>>> >>>>> >>>>> Either both should use target.source-map, or neither should. How do I >>>>> run new_context.line_entry.file through the target.source-map >>>>> normalization? >>>> >>>> >>>> >>>> It doesn't seem right to me that when symbols are handed out they have the >>>> source map applied to their file spec's. After all, then you could get >>>> into problems like: somebody started a step so we recorded m_addr_context. >>>> Then their step was interrupted (say by hitting a breakpoint) and the >>>> user added a source mapping. Then we stop in a frame from the same >>>> module, and now the SymbolContext that the step plan stored >>>> (m_addr_context) has a different path than the one in the frame when we >>>> get to it. Checking every time you compared file specs seems very error >>>> prone, we shouldn't do it that way. I guess if the FileSpec == handled >>>> this it would be odd but not too bad. But that seems like it would result >>>> in a lot of unnecessary work. I think it would be better to only do >>>> source map path conversion when sources are looked up, and maybe when >>>> paths are printed. For symbols we should stick to what the debug info >>>> says. >>>> >>>> Jim >>>> >>>> >>>>> >>>>> Ted >>>>> >>>>> -- >>>>> Qualcomm Innovation Center, Inc. >>>>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a >>>>> Linux Foundation Collaborative Project >>>>> >>>>> _______________________________________________ >>>>> lldb-dev mailing list >>>>> lldb-dev@lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >>>> >>>> >>> >>> _______________________________________________ >>> lldb-dev mailing list >>> lldb-dev@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >> >> > > _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
