Re: [GHC] #14999: unwinding info for stg_catch_frame is wrong

4 Apr 2018

      #14999: unwinding info for stg_catch_frame is wrong
-------------------------------------+-------------------------------------
        Reporter:  niteria           |                Owner:  niteria
            Type:  bug               |               Status:  new
        Priority:  normal            |            Milestone:  8.4.3
       Component:  Compiler          |              Version:
      Resolution:                    |             Keywords:
Operating System:  Linux             |         Architecture:  x86_64
 Type of failure:  Debugging         |  (amd64)
  information is incorrect           |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by niteria):

 First thing is to understand how to read unwind information in Cmm.

 If you have:
 {{{
 A // starts at addr X and ends at addr Y
 unwind Sp = Just Sp + 16;
 B // starts at addr Y and ends at addr Z
 }}}

 Does the unwind information reflect the state after `A` or `B`?
 If it reflects the state after `A` then you'd expect the LOC to be `Y`.
 If it reflects the state after `B` then you'd expect the LOC to be `Z`.

 I couldn't find a relevant note, so I resorted to experimentation.
 It appears that the unwind annotation reflects the state after `A`, but
 before `B`.

 With that in mind we can look at relevant Cmm dumps (`-ddump-cmm -ddump-
 opt-cmm -ddump-cmm-proc -ddump-cmm-verbose`):

 {{{

 ==================== Parsed Cmm ====================
 [stg_catch_frame_ret() //  [R1]
          { info_tbl: [(c5,
                        label: stg_catch_frame_info
                        rep:tag:34 StackRep [True, False])]
            stack_info: arg_space: 32 updfr_space: Just 8
          }
      {offset
        c5: // c1
            _c1::I64 = I64[(old + 32)];   // CmmAssign
            _c2::I64 = I64[(old + 24)];   // CmmAssign
            _c3::P64 = P64[(old + 16)];   // CmmAssign
            _c4::P64 = R1;   // CmmAssign
            //tick src<Small.cmm:(8,1)-(11,1)>
            unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
            R1 = _c4::P64;   // CmmAssign
            call (P64[(old + 8)])(R1) args: 8, res: 0, upd: 8;   // CmmCall
      }
  }]

 ==================== Post control-flow optimisations ====================
 {offset
   c5: // c1
       _c1::I64 = I64[(old + 32)];   // CmmAssign
       _c2::I64 = I64[(old + 24)];   // CmmAssign
       _c3::P64 = P64[(old + 16)];   // CmmAssign
       _c4::P64 = R1;   // CmmAssign
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       call (P64[(old + 8)])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== Post common block elimination ====================
 {offset
   c5: // c1
       _c1::I64 = I64[(old + 32)];   // CmmAssign
       _c2::I64 = I64[(old + 24)];   // CmmAssign
       _c3::P64 = P64[(old + 16)];   // CmmAssign
       _c4::P64 = R1;   // CmmAssign
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       call (P64[(old + 8)])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== Post switch plan ====================
 {offset
   c5: // c1
       _c1::I64 = I64[(old + 32)];   // CmmAssign
       _c2::I64 = I64[(old + 24)];   // CmmAssign
       _c3::P64 = P64[(old + 16)];   // CmmAssign
       _c4::P64 = R1;   // CmmAssign
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       call (P64[(old + 8)])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== Layout Stack ====================
 {offset
   c5: // c1
       unwind Sp = Just Sp + 24;   // CmmUnwind
       _c1::I64 = I64[Sp];   // CmmAssign
       _c2::I64 = I64[Sp + 8];   // CmmAssign
       _c3::P64 = P64[Sp + 16];   // CmmAssign
       _c4::P64 = R1;   // CmmAssign
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       unwind Sp = Just Sp + 0;   // CmmUnwind
       Sp = Sp + 24;   // CmmAssign
       call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== Sink assignments ====================
 {offset
   c5: // c1
       unwind Sp = Just Sp + 24;   // CmmUnwind
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + 24;   // CmmUnwind
       R1 = R1;   // CmmAssign
       unwind Sp = Just Sp;   // CmmUnwind
       Sp = Sp + 24;   // CmmAssign
       call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== Post common block elimination 2 ====================
 {offset
   c5: // c1
       unwind Sp = Just Sp + 24;   // CmmUnwind
       //tick src<Small.cmm:(8,1)-(11,1)>
       unwind Sp = Just Sp + 24;   // CmmUnwind
       R1 = R1;   // CmmAssign
       unwind Sp = Just Sp;   // CmmUnwind
       Sp = Sp + 24;   // CmmAssign
       call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
 }

 ==================== CAFEnv ====================
 [(c5, {})]

 ==================== after setInfoTableStackMap ====================
 stg_catch_frame_ret() //  [R1]
         { info_tbl: [(c5,
                       label: stg_catch_frame_info
                       rep:tag:34 StackRep [True, False])]
           stack_info: arg_space: 32 updfr_space: Just 8
         }
     {offset
       c5: // c1
           unwind Sp = Just Sp + 24;   // CmmUnwind
           //tick src<Small.cmm:(8,1)-(11,1)>
           unwind Sp = Just Sp + 24;   // CmmUnwind
           R1 = R1;   // CmmAssign
           unwind Sp = Just Sp;   // CmmUnwind
           Sp = Sp + 24;   // CmmAssign
           call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
     }
 }

 ==================== Post control-flow optimisations ====================
 stg_catch_frame_ret() //  [R1]
         { info_tbl: [(c5,
                       label: stg_catch_frame_info
                       rep:tag:34 StackRep [True, False])]
           stack_info: arg_space: 32 updfr_space: Just 8
         }
     {offset
       c5: // c1
           unwind Sp = Just Sp + 24;   // CmmUnwind
           //tick src<Small.cmm:(8,1)-(11,1)>
           unwind Sp = Just Sp + 24;   // CmmUnwind
           R1 = R1;   // CmmAssign
           unwind Sp = Just Sp;   // CmmUnwind
           Sp = Sp + 24;   // CmmAssign
           call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
     }
 }

 ==================== Post CPS Cmm ====================
 [stg_catch_frame_ret() //  [R1]
          { info_tbl: [(c5,
                        label: stg_catch_frame_info
                        rep:tag:34 StackRep [True, False])]
            stack_info: arg_space: 32 updfr_space: Just 8
          }
      {offset
        c5: // c1
            unwind Sp = Just Sp + 24;   // CmmUnwind
            //tick src<Small.cmm:(8,1)-(11,1)>
            unwind Sp = Just Sp + 24;   // CmmUnwind
            R1 = R1;   // CmmAssign
            unwind Sp = Just Sp;   // CmmUnwind
            Sp = Sp + 24;   // CmmAssign
            call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
      }
  }]

 ==================== Optimised Cmm ====================
 stg_catch_frame_ret() //  [R1]
         { [(c5,
             stg_catch_frame_info:
                 const 66;
                 const 34;)]
         }
     {offset
       c5: // c1
           unwind Sp = Just Sp + 24;   // CmmUnwind
           //tick src<Small.cmm:(8,1)-(11,1)>
           unwind Sp = Just Sp + 24;   // CmmUnwind
           // nop
           unwind Sp = Just Sp;   // CmmUnwind
           Sp = Sp + 24;   // CmmAssign
           call (P64[Sp])(R1) args: 8, res: 0, upd: 8;   // CmmCall
     }
 }
 }}}

 The first place where things go obviously wrong is `Layout Stack`.

 Let's focus on this fragment:

 {{{
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       unwind Sp = Just Sp + 0;   // CmmUnwind
       Sp = Sp + 24;   // CmmAssign
 }}}

 It looks like we swapped the unwind with the `Sp` update.
 Here's what we'd want:

 {{{
       unwind Sp = Just Sp + (8 + 16);   // CmmUnwind
       R1 = _c4::P64;   // CmmAssign
       Sp = Sp + 24;   // CmmAssign
       unwind Sp = Just Sp + 0;   // CmmUnwind
 }}}

 It turns out that the place that's responsible for this is `manifestSp` in
 `CmmLayoutStack`.

 After a small change:

 {{{
 diff --git a/compiler/cmm/CmmLayoutStack.hs
 b/compiler/cmm/CmmLayoutStack.hs
 index d2525d1..1e8c7c4 100644
 --- a/compiler/cmm/CmmLayoutStack.hs
 +++ b/compiler/cmm/CmmLayoutStack.hs
 @@ -860,7 +860,7 @@ manifestSp dflags stackmaps stack0 sp0 sp_high
        = block
        where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags)

 -    -- Add unwind pseudo-instruction right before the Sp adjustment
 +    -- Add unwind pseudo-instruction right after the Sp adjustment
      -- if there is one.
      add_adj_unwind block
        | debugLevel dflags > 0
 @@ -870,8 +870,9 @@ manifestSp dflags stackmaps stack0 sp0 sp_high
        = block
        where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags - sp_off)

 -    final_middle = maybeAddSpAdj dflags sp_off
 -                 . add_adj_unwind
 +    final_middle =
 +                 add_adj_unwind
 +                 . maybeAddSpAdj dflags sp_off
                   . add_initial_unwind
                   . blockFromList
                   . map adj_pre_sp
 }}}

 We get:

 {{{
 0000000000000010 <stg_catch_frame_info>:
   10:   48 83 c5 18             add    $0x18,%rbp
   14:   ff 65 00                jmpq   *0x0(%rbp)
 }}}

 {{{
 Contents of the .debug_frame section:

 00000000 0000000000000014 ffffffff CIE "" cf=1 df=-8 ra=16
    LOC           CFA      rbp   rsp   ra
 0000000000000000 rbp+0    v+0   s     c+0

 00000018 000000000000002c 00000000 FDE cie=00000000
 pc=000000000000000f..0000000000000017
    LOC           CFA      rbp   rsp   ra
 000000000000000f rbp+0    v+0   s     c+0
 000000000000000f rbp+24   v+0   s     c+0
 0000000000000014 rbp+0    v+0   s     c+0
 }}}

 As a side note, the manually inserted unwind info in `stg_catch_frame`
 appears to be redundant. Layout Stack appears to be doing the same thing.

 5279b08ba3f01e4b7e28d12b2751413d789d9fbe is what introduced this behavior,
 so I'd like Ben to check if I didn't go off the rails somewhere.

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14999#comment:2>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler