GaloisInc / macaw

Open source binary analysis tools.
BSD 3-Clause "New" or "Revised" License
208 stars 21 forks source link

ppc-symbolic: `Index`es for registers #445

Closed langston-barrett closed 1 month ago

langston-barrett commented 2 months ago

Like #444, but for PPC.

langston-barrett commented 1 month ago

Huh, GHC 9.8 yields:

Error: cabal: Failed to build macaw-ppc-symbolic-0.1.0.0 (which is required by
test:macaw-ppc-symbolic-tests from macaw-ppc-symbolic-0.1.0.0). The build
process was killed (i.e. SIGKILL). The typical reason for this is that there
is not enough memory available (e.g. the OS killed a process using lots of
memory).
RyanGlScott commented 1 month ago

Wow. I thought maybe that this was a CI fluke, but no: it's very reproducible. I can even reproduce it locally with my (fairly powerful) laptop.

The problem arises when compiling the MS.updateReg = archUpdateReg fields of GenArchVals in the GenArchInfo instnaces for AnyPPC V32 or AnyPPC V64 in Data.Macaw.Symbolic.PPC. Interestingly, if you mark Regs.updateReg as INLINE:

diff --git a/macaw-ppc-symbolic/src/Data/Macaw/PPC/Symbolic/Regs.hs b/macaw-ppc-symbolic/src/Data/Macaw/PPC/Symbolic/Regs.hs
index 3aeafb81..0931ff40 100644
--- a/macaw-ppc-symbolic/src/Data/Macaw/PPC/Symbolic/Regs.hs
+++ b/macaw-ppc-symbolic/src/Data/Macaw/PPC/Symbolic/Regs.hs
@@ -427,6 +427,7 @@ lookupReg r asgn =
     Nothing -> X.throwM (MissingRegisterInState (Some r))
     Just pair -> return (asgn Ctx.! MSB.crucibleIndex pair)

+{-# INLINE updateReg #-}
 updateReg :: forall v ppc m f tp
            . (MP.KnownVariant v,
                ppc ~ MP.AnyPPC v,

Then the problem goes away. Hm...

langston-barrett commented 1 month ago

Yeah, I also see this locally with GHC 9.8. Seems like a regression in the compiler?

langston-barrett commented 1 month ago

@RyanGlScott Are you good with merging this with the INLINE workaround, or would you like for us to do some additional digging into what exactly caused this compiler performance issue?

RyanGlScott commented 1 month ago

I did spend some time attempting to minimize the issue, but I was ultimately unsuccessful. I do think that this is worth reporting upstream to GHC, so I've opened https://gitlab.haskell.org/ghc/ghc/-/issues/25301 to track this. That being said, I don't think we should wait until that is fixed before proceeding.

Could you cite this GHC issue in the comments next to updateReg? Then I think this would be ready to land.