Hi Stella,

The logs are really helpful, thanks. This part is unexpected:

python           Finding frames between main and sink(), retn-pc=0x4005b8
python           GetCallEdges: Attempting to parse call site info for main
python           CollectCallEdges: Found call site info in main
python           CollectCallEdges: Found call origin: _Z5func2v (retn-PC: 
0x4005b8)
python           CollectCallEdges: Found call origin: _Z5func1v (retn-PC: 
0x4005c2)
python           FindInterveningFrames: found call with retn-PC = 0x800a38
python           FindInterveningFrames: found call with retn-PC = 0x800a42

LLDB finds a call from main() with return PC = 0x4005b8. It’s able to parse the 
call site info within main’s debug info. It finds a call from main() into func2 
with that exact return PC. But, the address calculation in 
CallEdge::GetReturnPCAddress adds the wrong slide to this return PC, giving 
0x800a38. This doesn’t match the PC value in the register state, so lldb can’t 
create a tail call frame.

I think Address::GetLoadAddress is the right API to use, but it’s clearly not 
achieving the right result here. I’ll experiment with replacing return PC 
addresses with function-local offsets to the instruction after a call. The idea 
would be to simply add this offset to the base address of the function, instead 
of doing a load address calculation.

vedant

> On Oct 18, 2018, at 9:47 AM, Stella Stamenova <sti...@microsoft.com> wrote:
> 
> Hey Vedant,
>  
> I’ve attached the logs from Linux.
>  
> Most of the tests now pass on Windows with the exception of 
> TestSteppingOutWithArtificialFrames and TestTailCallFrameSBAPI. Both of these 
> attempt to get a specific frame by calling GetFrameAtIndex which only works 
> partially on Windows right now. I think we should mark these as XFAIL on 
> Windows and link them to: https://bugs.llvm.org/show_bug.cgi?id=26265 
> <https://bugs.llvm.org/show_bug.cgi?id=26265>.
>  
> Thanks,
> -Stella
>  
> From: v...@apple.com <mailto:v...@apple.com> <v...@apple.com 
> <mailto:v...@apple.com>> 
> Sent: Tuesday, October 16, 2018 11:17 AM
> To: Stella Stamenova <sti...@microsoft.com <mailto:sti...@microsoft.com>>
> Cc: Frédéric Riss <fr...@apple.com <mailto:fr...@apple.com>>; 
> reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
> <mailto:reviews+d50478+public+7e86b794a0909...@reviews.llvm.org>; Adrian 
> Prantl <apra...@apple.com <mailto:apra...@apple.com>>; paul.robin...@sony.com 
> <mailto:paul.robin...@sony.com>; jdevliegh...@apple.com 
> <mailto:jdevliegh...@apple.com>; Jim Ingham <jing...@apple.com 
> <mailto:jing...@apple.com>>; ztur...@google.com <mailto:ztur...@google.com>; 
> abidh....@gmail.com <mailto:abidh....@gmail.com>; teempe...@gmail.com 
> <mailto:teempe...@gmail.com>; sgraen...@apple.com 
> <mailto:sgraen...@apple.com>; mgr...@codeaurora.org 
> <mailto:mgr...@codeaurora.org>; dblai...@gmail.com 
> <mailto:dblai...@gmail.com>; lldb-commits@lists.llvm.org 
> <mailto:lldb-commits@lists.llvm.org>
> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
>  
>  
> 
> 
> On Oct 16, 2018, at 10:59 AM, Stella Stamenova <sti...@microsoft.com 
> <mailto:sti...@microsoft.com>> wrote:
>  
> The windows error is because the names are different, as you expected:
> AssertionError: 'void sink(void)' != 'sink()'
> You can probably update the test to look for a different name on Windows 
> (though if I recall correctly, different versions of the DIA sdk provide 
> different detail on the names, so that might not be robust either) or look 
> for a substring in the full name.
>  
> I used a substring check in r344634.
>  
> 
> 
> I’ll look into the Linux error as well and let you know what I find.
>  
> Thank you very much! I really appreciate your help and patience with this.
>  
> The "step" logging channel should provide detailed information about what 
> goes wrong when parsing the DWARF for call site information and creating 
> artificial frames.
>  
> vedant
> 
> 
> From: v...@apple.com <mailto:v...@apple.com> <v...@apple.com 
> <mailto:v...@apple.com>> 
> Sent: Monday, October 15, 2018 8:34 PM
> To: Frédéric Riss <fr...@apple.com <mailto:fr...@apple.com>>
> Cc: reviews+d50478+public+7e86b794a0909...@reviews.llvm.org 
> <mailto:reviews+d50478+public+7e86b794a0909...@reviews.llvm.org>; Adrian 
> Prantl <apra...@apple.com <mailto:apra...@apple.com>>; paul.robin...@sony.com 
> <mailto:paul.robin...@sony.com>; jdevliegh...@apple.com 
> <mailto:jdevliegh...@apple.com>; Jim Ingham <jing...@apple.com 
> <mailto:jing...@apple.com>>; ztur...@google.com <mailto:ztur...@google.com>; 
> Stella Stamenova <sti...@microsoft.com <mailto:sti...@microsoft.com>>; 
> abidh....@gmail.com <mailto:abidh....@gmail.com>; teempe...@gmail.com 
> <mailto:teempe...@gmail.com>; sgraen...@apple.com 
> <mailto:sgraen...@apple.com>; mgr...@codeaurora.org 
> <mailto:mgr...@codeaurora.org>; dblai...@gmail.com 
> <mailto:dblai...@gmail.com>; lldb-commits@lists.llvm.org 
> <mailto:lldb-commits@lists.llvm.org>
> Subject: Re: [PATCH] D50478: Add support for artificial tail call frames
> 
> 
> 
> On Oct 15, 2018, at 4:46 PM, Frédéric Riss <fr...@apple.com 
> <mailto:fr...@apple.com>> wrote:
> 
> 
> 
> 
> On Oct 15, 2018, at 4:40 PM, Vedant Kumar <v...@apple.com 
> <mailto:v...@apple.com>> wrote:
> 
> 
> 
> 
> On Oct 15, 2018, at 3:47 PM, Stella Stamenova via Phabricator 
> <revi...@reviews.llvm.org <mailto:revi...@reviews.llvm.org>> wrote:
> 
> stella.stamenova added a comment.
> 
> In https://reviews.llvm.org/D50478#1262717 
> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD50478%231262717&data=02%7C01%7Cstilis%40microsoft.com%7Cf9216ae492894050d92c08d633939d6f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753106442955534&sdata=B7OOidlsIkojfOmNrwDf77eFvcMGnusASMyjrYa8lEI%3D&reserved=0>,
>  @vsk wrote:
> 
> 
> 
> In https://reviews.llvm.org/D50478#1262710 
> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD50478%231262710&data=02%7C01%7Cstilis%40microsoft.com%7Cf9216ae492894050d92c08d633939d6f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753106442955534&sdata=%2BdQxQwN%2B5svfM%2FFNvOR%2FpUhf3lArVs%2FEeeshtYk2qsM%3D&reserved=0>,
>  @stella.stamenova wrote:
> 
> 
> 
> Unfortunately, the bots are broken because of the FileCheck issue, so I can't 
> confirm with them, but I see a number of these tests fail in our local 
> testing. Some fail on both Windows and Linux and some just fail on Linux. 
> Here are the failing tests:
> 
> Linux:
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
> lldb-Suite :: 
> functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
> lldb-Suite :: 
> functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
> lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
> lldb-Suite :: 
> functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
> 
> Windows:
> lldb-Suite :: 
> functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
> lldb-Suite :: 
> functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
> 
> 
> Let me know what you need to investigate.
> 
> 
> Strange, I didn't get any bot failure notifications in the days after this 
> landed. Could you share the output from the failing tests?
> 
> 
> All the failures on Windows are happening when validating the function name. 
> For example:
> 
> ======================================================================
> 
> FAIL: test_tail_call_frame_sbapi 
> (TestTailCallFrameSBAPI.TestTailCallFrameSBAPI)
> 
> ----------------------------------------------------------------------
> 
> Traceback (most recent call last):
> 
> File 
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
>  line 19, in test_tail_call_frame_sbapi
> 
> self.do_test()
> 
> File 
> "E:\_work\55\s\llvm\tools\lldb\packages\Python\lldbsuite\test\functionalities\tail_call_frames\sbapi_support\TestTailCallFrameSBAPI.py",
>  line 64, in do_test
> 
> self.assertTrue(frame.GetDisplayFunctionName() == name)
> 
> It could be that the display name of a function is formatted differently on 
> Windows. Do you have an easy way of determining what 
> frame.GetDisplayFunctionName() is?
> If you use assertEqual(a,b) instead of assertTrue, it will print out the 
> values and make it easier to debug.
> Thanks, done in r344581.
> vedant
> 
> 
> 
> Fred
> 
> 
> 
> 
> 
> 
> 
> AssertionError: False is not True
> 
> Config=x86_64-E:\_work\55\b\LLVMBuild\Release\bin\clang.exe
> 
> ----------------------------------------------------------------------
> 
> There are several different failures on Linux. Here's the first one:
> 
> FAIL: LLDB (/vstsdrive/_work/38/b/LLVMBuild/bin/clang-8-x86_64) :: test_dwarf 
> (lldbsuite.test.lldbtest.TestDisambiguateCallSite)
> 
> --- FileCheck trace (code=1) ---
> /vstsdrive/_work/38/b/LLVMBuild/bin/FileCheck 
> /vstsdrive/_work/38/s/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
>  -implicit-check-not=artificial
> 
> FileCheck input:
> * thread #1, name = 'a.out', stop reason = breakpoint 1.1
> * frame #0: 0x0000000000400580 a.out`sink() at main.cpp:13:4 [opt]
> frame #1: 0x00000000004005b8 a.out`main(argc=1, (null)=) at main.cpp:28:3 
> [opt]
> frame #2: 0x00007f980aff7830 libc.so.6`__libc_start_main + 240
> frame #3: 0x00000000004004a9 a.out`_start + 41
> 
> It looks like we're not generating tail call frames on Linux at all. It would 
> help to have logs from "log enable -f /tmp/linux-stepping.log lldb step".
> 
> I'm headed out of the office now, but If you need to disable the tests on 
> Windows/Linux , the fastest way to do that would be to add a platform check 
> to skipUnlessHasCallSiteInfo in decorators.py.
> 
> vedant
> 
> 
> 
> 
> 
> 
> FileCheck output:
> 
> /vstsdrive/_work/38/s/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp:15:17:
>  error: CHECK-NEXT: expected string not found in input
> // CHECK-NEXT: func2{{.*}} [opt] [artificial]
> ^
> :3:2: note: scanning from here
> frame #1: 0x00000000004005b8 a.out`main(argc=1, (null)=) at main.cpp:28:3 
> [opt]
> ^
> :3:80: note: possible intended match here
> frame #1: 0x00000000004005b8 a.out`main(argc=1, (null)=) at main.cpp:28:3 
> [opt]
> ^
> 
> Let me know if you need more logs.
> 
> 
> Repository:
> rLLDB LLDB
> 
> https://reviews.llvm.org/D50478 
> <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD50478&data=02%7C01%7Cstilis%40microsoft.com%7Cf9216ae492894050d92c08d633939d6f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636753106442955534&sdata=NtVhAL0OUf26pOdcD2EdILhwqG8Mfgrc3kEEywrZ5as%3D&reserved=0>
>  
> <linux-stepping.log>

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to