From e8f7734d8a052f99b03e1123466dc9f47b48c311 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 10 Jun 2021 23:14:35 +0000 Subject: Fix #19931 The issue was the renderer for x86 addressing modes assumes native size registers, but we were passing in a possibly-smaller index in conjunction with a native-sized base pointer. The easist thing to do is just extend the register first. I also changed the other NGC backends implementing jump tables accordingly. On one hand, I think PowerPC and Sparc don't have the small sub-registers anyways so there is less to worry about. On the other hand, to the extent that's true the zero extension can become a no-op. I should give credit where it's due: @hsyl20 really did all the work for me in https://gitlab.haskell.org/ghc/ghc/-/merge_requests/4717#note_355874, but I was daft and missed the "Oops" and so ended up spending a silly amount of time putting it all back together myself. The unregisterised backend change is a bit different, because here we are translating the actual case not a jump table, and the fix is to handle right-sized literals not addressing modes. But it makes sense to include here too because it's the same change in the subsequent commit that exposes both bugs. --- compiler/GHC/CmmToAsm/PPC/CodeGen.hs | 12 +++++++++--- compiler/GHC/CmmToAsm/SPARC/CodeGen.hs | 12 ++++++++++-- compiler/GHC/CmmToAsm/X86/CodeGen.hs | 11 +++++++++-- 3 files changed, 28 insertions(+), 7 deletions(-) (limited to 'compiler/GHC/CmmToAsm') diff --git a/compiler/GHC/CmmToAsm/PPC/CodeGen.hs b/compiler/GHC/CmmToAsm/PPC/CodeGen.hs index 8ee20e06f5..67bc3d9bdb 100644 --- a/compiler/GHC/CmmToAsm/PPC/CodeGen.hs +++ b/compiler/GHC/CmmToAsm/PPC/CodeGen.hs @@ -2086,7 +2086,7 @@ genSwitch :: NCGConfig -> CmmExpr -> SwitchTargets -> NatM InstrBlock genSwitch config expr targets | OSAIX <- platformOS platform = do - (reg,e_code) <- getSomeReg (cmmOffset platform expr offset) + (reg,e_code) <- getSomeReg indexExpr let fmt = archWordFormat $ target32Bit platform sha = if target32Bit platform then 2 else 3 tmp <- getNewRegNat fmt @@ -2103,7 +2103,7 @@ genSwitch config expr targets | (ncgPIC config) || (not $ target32Bit platform) = do - (reg,e_code) <- getSomeReg (cmmOffset platform expr offset) + (reg,e_code) <- getSomeReg indexExpr let fmt = archWordFormat $ target32Bit platform sha = if target32Bit platform then 2 else 3 tmp <- getNewRegNat fmt @@ -2120,7 +2120,7 @@ genSwitch config expr targets return code | otherwise = do - (reg,e_code) <- getSomeReg (cmmOffset platform expr offset) + (reg,e_code) <- getSomeReg indexExpr let fmt = archWordFormat $ target32Bit platform sha = if target32Bit platform then 2 else 3 tmp <- getNewRegNat fmt @@ -2134,6 +2134,12 @@ genSwitch config expr targets ] return code where + indexExpr = cmmOffset platform exprWidened offset + -- We widen to a native-width register to santize the high bits + exprWidened = CmmMachOp + (MO_UU_Conv (cmmExprWidth platform expr) + (platformWordWidth platform)) + [expr] (offset, ids) = switchTargetsToTable targets platform = ncgPlatform config diff --git a/compiler/GHC/CmmToAsm/SPARC/CodeGen.hs b/compiler/GHC/CmmToAsm/SPARC/CodeGen.hs index 56f764560c..aeaaf1c9d3 100644 --- a/compiler/GHC/CmmToAsm/SPARC/CodeGen.hs +++ b/compiler/GHC/CmmToAsm/SPARC/CodeGen.hs @@ -313,7 +313,7 @@ genSwitch config expr targets = error "MachCodeGen: sparc genSwitch PIC not finished\n" | otherwise - = do (e_reg, e_code) <- getSomeReg (cmmOffset (ncgPlatform config) expr offset) + = do (e_reg, e_code) <- getSomeReg indexExpr base_reg <- getNewRegNat II32 offset_reg <- getNewRegNat II32 @@ -334,7 +334,15 @@ genSwitch config expr targets , LD II32 (AddrRegReg base_reg offset_reg) dst , JMP_TBL (AddrRegImm dst (ImmInt 0)) ids label , NOP ] - where (offset, ids) = switchTargetsToTable targets + where + indexExpr = cmmOffset platform exprWidened offset + -- We widen to a native-width register to santize the high bits + exprWidened = CmmMachOp + (MO_UU_Conv (cmmExprWidth platform expr) + (platformWordWidth platform)) + [expr] + (offset, ids) = switchTargetsToTable targets + platform = ncgPlatform config generateJumpTableForInstr :: Platform -> Instr -> Maybe (NatCmmDecl RawCmmStatics Instr) diff --git a/compiler/GHC/CmmToAsm/X86/CodeGen.hs b/compiler/GHC/CmmToAsm/X86/CodeGen.hs index ff25a2e53f..5e7c261cbb 100644 --- a/compiler/GHC/CmmToAsm/X86/CodeGen.hs +++ b/compiler/GHC/CmmToAsm/X86/CodeGen.hs @@ -3476,9 +3476,16 @@ genSwitch :: CmmExpr -> SwitchTargets -> NatM InstrBlock genSwitch expr targets = do config <- getConfig let platform = ncgPlatform config + -- We widen to a native-width register because we cannot use arbitry sizes + -- in x86 addressing modes. + exprWidened = CmmMachOp + (MO_UU_Conv (cmmExprWidth platform expr) + (platformWordWidth platform)) + [expr] + indexExpr = cmmOffset platform exprWidened offset if ncgPIC config then do - (reg,e_code) <- getNonClobberedReg (cmmOffset platform expr offset) + (reg,e_code) <- getNonClobberedReg indexExpr -- getNonClobberedReg because it needs to survive across t_code lbl <- getNewLabelNat let is32bit = target32Bit platform @@ -3519,7 +3526,7 @@ genSwitch expr targets = do JMP_TBL (OpReg tableReg) ids rosection lbl ] else do - (reg,e_code) <- getSomeReg (cmmOffset platform expr offset) + (reg,e_code) <- getSomeReg indexExpr lbl <- getNewLabelNat let op = OpAddr (AddrBaseIndex EABaseNone (EAIndex reg (platformWordSizeInBytes platform)) (ImmCLbl lbl)) code = e_code `appOL` toOL [ -- cgit v1.2.1