Commit 6132d7c5 authored by niteria's avatar niteria Committed by Ben Gamari

Correctly add unwinding info in manifestSp and makeFixupBlocks

In `manifestSp` the unwind info was before the relevant instruction, not
after.  I added some notes to establish semantics.  Also removes
redundant annotation in stg_catch_frame.

For `makeFixupBlocks` it looks like we were off by `wORD_SIZE dflags`.
I'm not sure why, but it lines up with `manifestSp`.  In fact it lines
up so well so that I can consolidate the Sp unwind logic in
`maybeAddUnwind`.  I detected the problems with `makeFixupBlocks` by
running T14779b after patching D4559.

Test Plan: added a new test

Reviewers: bgamari, scpmw, simonmar, erikd

Reviewed By: bgamari

Subscribers: thomie, carter

GHC Trac Issues: #14999

Differential Revision: https://phabricator.haskell.org/D4606
parent 75361b11
...@@ -577,15 +577,8 @@ makeFixupBlock dflags sp0 l stack tscope assigs ...@@ -577,15 +577,8 @@ makeFixupBlock dflags sp0 l stack tscope assigs
| otherwise = do | otherwise = do
tmp_lbl <- newBlockId tmp_lbl <- newBlockId
let sp_off = sp0 - sm_sp stack let sp_off = sp0 - sm_sp stack
maybeAddUnwind block
| debugLevel dflags > 0
= block `blockSnoc` CmmUnwind [(Sp, Just unwind_val)]
| otherwise
= block
where unwind_val = cmmOffset dflags spExpr (sm_sp stack)
block = blockJoin (CmmEntry tmp_lbl tscope) block = blockJoin (CmmEntry tmp_lbl tscope)
( maybeAddSpAdj dflags sp_off ( maybeAddSpAdj dflags sp0 sp_off
$ maybeAddUnwind
$ blockFromList assigs ) $ blockFromList assigs )
(CmmBranch l) (CmmBranch l)
return (tmp_lbl, [block]) return (tmp_lbl, [block])
...@@ -851,28 +844,7 @@ manifestSp dflags stackmaps stack0 sp0 sp_high ...@@ -851,28 +844,7 @@ manifestSp dflags stackmaps stack0 sp0 sp_high
adj_pre_sp = mapExpDeep (areaToSp dflags sp0 sp_high area_off) adj_pre_sp = mapExpDeep (areaToSp dflags sp0 sp_high area_off)
adj_post_sp = mapExpDeep (areaToSp dflags (sp0 - sp_off) sp_high area_off) adj_post_sp = mapExpDeep (areaToSp dflags (sp0 - sp_off) sp_high area_off)
-- Add unwind pseudo-instruction at the beginning of each block to final_middle = maybeAddSpAdj dflags sp0 sp_off
-- document Sp level for debugging
add_initial_unwind block
| debugLevel dflags > 0
= CmmUnwind [(Sp, Just sp_unwind)] `blockCons` block
| otherwise
= block
where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags)
-- Add unwind pseudo-instruction right before the Sp adjustment
-- if there is one.
add_adj_unwind block
| debugLevel dflags > 0
, sp_off /= 0
= block `blockSnoc` CmmUnwind [(Sp, Just sp_unwind)]
| otherwise
= block
where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags - sp_off)
final_middle = maybeAddSpAdj dflags sp_off
. add_adj_unwind
. add_initial_unwind
. blockFromList . blockFromList
. map adj_pre_sp . map adj_pre_sp
. elimStackStores stack0 stackmaps area_off . elimStackStores stack0 stackmaps area_off
...@@ -891,11 +863,33 @@ getAreaOff stackmaps (Young l) = ...@@ -891,11 +863,33 @@ getAreaOff stackmaps (Young l) =
Nothing -> pprPanic "getAreaOff" (ppr l) Nothing -> pprPanic "getAreaOff" (ppr l)
maybeAddSpAdj :: DynFlags -> ByteOff -> Block CmmNode O O -> Block CmmNode O O maybeAddSpAdj
maybeAddSpAdj _ 0 block = block :: DynFlags -> ByteOff -> ByteOff -> Block CmmNode O O -> Block CmmNode O O
maybeAddSpAdj dflags sp_off block = block `blockSnoc` adj maybeAddSpAdj dflags sp0 sp_off block =
add_initial_unwind $ add_adj_unwind $ adj block
where where
adj = CmmAssign spReg (cmmOffset dflags spExpr sp_off) adj block
| sp_off /= 0
= block `blockSnoc` CmmAssign spReg (cmmOffset dflags spExpr sp_off)
| otherwise = block
-- Add unwind pseudo-instruction at the beginning of each block to
-- document Sp level for debugging
add_initial_unwind block
| debugLevel dflags > 0
= CmmUnwind [(Sp, Just sp_unwind)] `blockCons` block
| otherwise
= block
where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags)
-- Add unwind pseudo-instruction right after the Sp adjustment
-- if there is one.
add_adj_unwind block
| debugLevel dflags > 0
, sp_off /= 0
= block `blockSnoc` CmmUnwind [(Sp, Just sp_unwind)]
| otherwise
= block
where sp_unwind = CmmRegOff spReg (sp0 - wORD_SIZE dflags - sp_off)
{- Note [SP old/young offsets] {- Note [SP old/young offsets]
......
...@@ -349,8 +349,8 @@ in addition to the usual beginning-of-block statement, ...@@ -349,8 +349,8 @@ in addition to the usual beginning-of-block statement,
unwind Sp = Just Sp + 0; unwind Sp = Just Sp + 0;
I64[Sp - 8] = c2dD; I64[Sp - 8] = c2dD;
R1 = v :: P64; R1 = v :: P64;
unwind Sp = Just Sp + 8;
Sp = Sp - 8; Sp = Sp - 8;
unwind Sp = Just Sp + 8;
if (R1 & 7 != 0) goto c2dD; else goto c2dE; if (R1 & 7 != 0) goto c2dD; else goto c2dE;
The remaining blocks are simple, The remaining blocks are simple,
...@@ -392,10 +392,95 @@ The flow of unwinding information through the compiler is a bit convoluted: ...@@ -392,10 +392,95 @@ The flow of unwinding information through the compiler is a bit convoluted:
* This unwind information is converted to DebugBlocks by Debug.cmmDebugGen * This unwind information is converted to DebugBlocks by Debug.cmmDebugGen
* These DebugBlcosk are then converted to, e.g., DWARF unwinding tables * These DebugBlocks are then converted to, e.g., DWARF unwinding tables
(by the Dwarf module) and emitted in the final object. (by the Dwarf module) and emitted in the final object.
See also: Note [Unwinding information in the NCG] in AsmCodeGen. See also:
Note [Unwinding information in the NCG] in AsmCodeGen,
Note [Unwind pseudo-instruction in Cmm],
Note [Debugging DWARF unwinding info].
Note [Debugging DWARF unwinding info]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
For debugging generated unwinding info I've found it most useful to dump the
disassembled binary with objdump -D and dump the debug info with
readelf --debug-dump=frames-interp.
You should get something like this:
0000000000000010 <stg_catch_frame_info>:
10: 48 83 c5 18 add $0x18,%rbp
14: ff 65 00 jmpq *0x0(%rbp)
and:
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 0000000000000024 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
To read it http://www.dwarfstd.org/doc/dwarf-2.0.0.pdf has a nice example in
Appendix 5 (page 101 of the pdf) and more details in the relevant section.
The key thing to keep in mind is that the value at LOC is the value from
*before* the instruction at LOC executes. In other words it answers the
question: if my $rip is at LOC, how do I get the relevant values given the
values obtained through unwinding so far.
If the readelf --debug-dump=frames-interp output looks wrong, it may also be
useful to look at readelf --debug-dump=frames, which is closer to the
information that GHC generated.
It's also useful to dump the relevant Cmm with -ddump-cmm -ddump-opt-cmm
-ddump-cmm-proc -ddump-cmm-verbose. Note [Unwind pseudo-instruction in Cmm]
explains how to interpret it.
Inside gdb there are a couple useful commands for inspecting frames.
For example:
gdb> info frame <num>
It shows the values of registers obtained through unwinding.
Another useful thing to try when debugging the DWARF unwinding is to enable
extra debugging output in GDB:
gdb> set debug frame 1
This makes GDB produce a trace of its internal workings. Having gone this far,
it's just a tiny step to run GDB in GDB. Make sure you install debugging
symbols for gdb if you obtain it through a package manager.
Keep in mind that the current release of GDB has an instruction pointer handling
heuristic that works well for C-like languages, but doesn't always work for
Haskell. See Note [Info Offset] in Dwarf.Types for more details.
Note [Unwind pseudo-instruction in Cmm]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
One of the possible CmmNodes is a CmmUnwind pseudo-instruction. It doesn't
generate any assembly, but controls what DWARF unwinding information gets
generated.
It's important to understand what ranges of code the unwind pseudo-instruction
refers to.
For a sequence of CmmNodes like:
A // starts at addr X and ends at addr Y-1
unwind Sp = Just Sp + 16;
B // starts at addr Y and ends at addr Z
the unwind statement reflects the state after A has executed, but before B
has executed. If you consult the Note [Debugging DWARF unwinding info], the
LOC this information will end up in is Y.
-} -}
-- | A label associated with an 'UnwindTable' -- | A label associated with an 'UnwindTable'
......
...@@ -415,6 +415,12 @@ pprFrameBlock (DwarfFrameBlock hasInfo uws0) = ...@@ -415,6 +415,12 @@ pprFrameBlock (DwarfFrameBlock hasInfo uws0) =
-- Note that this will not prevent GDB from failing to look-up the -- Note that this will not prevent GDB from failing to look-up the
-- correct function name for the frame, as that uses the symbol table, -- correct function name for the frame, as that uses the symbol table,
-- which we can not manipulate as easily. -- which we can not manipulate as easily.
--
-- There's a GDB patch to address this at [1]. At the moment of writing
-- it's not merged, so I recommend building GDB with the patch if you
-- care about unwinding. The hack above doesn't cover every case.
--
-- [1] https://sourceware.org/ml/gdb-patches/2018-02/msg00055.html
-- | Get DWARF register ID for a given GlobalReg -- | Get DWARF register ID for a given GlobalReg
dwarfGlobalRegNo :: Platform -> GlobalReg -> Word8 dwarfGlobalRegNo :: Platform -> GlobalReg -> Word8
......
...@@ -370,7 +370,6 @@ INFO_TABLE_RET(stg_catch_frame, CATCH_FRAME, ...@@ -370,7 +370,6 @@ INFO_TABLE_RET(stg_catch_frame, CATCH_FRAME,
exceptions_blocked,handler)) exceptions_blocked,handler))
return (P_ ret) return (P_ ret)
{ {
unwind Sp = Sp + SIZEOF_StgCatchFrame;
return (ret); return (ret);
} }
......
...@@ -105,6 +105,12 @@ class TestConfig: ...@@ -105,6 +105,12 @@ class TestConfig:
# Do we have SMP support? # Do we have SMP support?
self.have_smp = False self.have_smp = False
# Is gdb avaliable?
self.have_gdb = False
# Is readelf available?
self.have_readelf = False
# Are we testing an in-tree compiler? # Are we testing an in-tree compiler?
self.in_tree_compiler = True self.in_tree_compiler = True
......
...@@ -413,6 +413,12 @@ def compiler_profiled( ): ...@@ -413,6 +413,12 @@ def compiler_profiled( ):
def compiler_debugged( ): def compiler_debugged( ):
return config.compiler_debugged return config.compiler_debugged
def have_gdb( ):
return config.have_gdb
def have_readelf( ):
return config.have_readelf
# --- # ---
def high_memory_usage(name, opts): def high_memory_usage(name, opts):
......
...@@ -90,6 +90,8 @@ GHC_PRIM_LIBDIR := $(subst library-dirs: ,,$(shell "$(GHC_PKG)" field ghc-prim l ...@@ -90,6 +90,8 @@ GHC_PRIM_LIBDIR := $(subst library-dirs: ,,$(shell "$(GHC_PKG)" field ghc-prim l
HAVE_VANILLA := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.hi ]; then echo YES; else echo NO; fi) HAVE_VANILLA := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.hi ]; then echo YES; else echo NO; fi)
HAVE_DYNAMIC := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.dyn_hi ]; then echo YES; else echo NO; fi) HAVE_DYNAMIC := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.dyn_hi ]; then echo YES; else echo NO; fi)
HAVE_PROFILING := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.p_hi ]; then echo YES; else echo NO; fi) HAVE_PROFILING := $(shell if [ -f $(subst \,/,$(GHC_PRIM_LIBDIR))/GHC/PrimopWrappers.p_hi ]; then echo YES; else echo NO; fi)
HAVE_GDB := $(shell if gdb --version > /dev/null 2> /dev/null; then echo YES; else echo NO; fi)
HAVE_READELF := $(shell if readelf --version > /dev/null 2> /dev/null; then echo YES; else echo NO; fi)
ifeq "$(HAVE_VANILLA)" "YES" ifeq "$(HAVE_VANILLA)" "YES"
RUNTEST_OPTS += -e config.have_vanilla=True RUNTEST_OPTS += -e config.have_vanilla=True
...@@ -135,6 +137,18 @@ else ...@@ -135,6 +137,18 @@ else
RUNTEST_OPTS += -e config.unregisterised=False RUNTEST_OPTS += -e config.unregisterised=False
endif endif
ifeq "$(HAVE_GDB)" "YES"
RUNTEST_OPTS += -e config.have_gdb=True
else
RUNTEST_OPTS += -e config.have_gdb=False
endif
ifeq "$(HAVE_READELF)" "YES"
RUNTEST_OPTS += -e config.have_readelf=True
else
RUNTEST_OPTS += -e config.have_readelf=False
endif
ifeq "$(GhcDynamicByDefault)" "YES" ifeq "$(GhcDynamicByDefault)" "YES"
RUNTEST_OPTS += -e config.ghc_dynamic_by_default=True RUNTEST_OPTS += -e config.ghc_dynamic_by_default=True
CABAL_MINIMAL_BUILD = --enable-shared --disable-library-vanilla CABAL_MINIMAL_BUILD = --enable-shared --disable-library-vanilla
......
...@@ -33,3 +33,8 @@ debug: ...@@ -33,3 +33,8 @@ debug:
./debug ./debug
rm debug rm debug
T14999:
'$(TEST_HC)' $(TEST_HC_OPTS) -O2 -g -c T14999.cmm -o T14999.o
gdb --batch -ex 'file T14999.o' -ex 'disassemble stg_catch_frame_info' --nx | tr -s '[:blank:]'
readelf --debug-dump=frames-interp T14999.o | tr -s '[:blank:]'
#define CATCH_FRAME 34
#define SIZEOF_StgCatchFrame (SIZEOF_StgHeader+16)
INFO_TABLE_RET(stg_catch_frame, CATCH_FRAME,
bits64 info_ptr, bits64 exceptions_blocked, gcptr handler)
return (gcptr ret)
{
return (ret);
}
Dump of assembler code for function stg_catch_frame_info:
0x0000000000000010 <+0>: add $0x18,%rbp
0x0000000000000014 <+4>: jmpq *0x0(%rbp)
End of assembler dump.
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
...@@ -38,3 +38,8 @@ test('T12355', normal, compile, ['']) ...@@ -38,3 +38,8 @@ test('T12355', normal, compile, [''])
test('T14626', test('T14626',
normal, normal,
run_command, ['$MAKE -s --no-print-directory T14626']) run_command, ['$MAKE -s --no-print-directory T14626'])
test('T14999',
[when((arch('powerpc64') or arch('powerpc64le')), expect_broken(11261)),
unless(opsys('linux') and arch('x86_64') and have_gdb() and
have_readelf(), skip)],
run_command, ['$MAKE -s --no-print-directory T14999'])
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment