ndmitchell / hoogle

Haskell API search engine
http://hoogle.haskell.org/
Other
738 stars 134 forks source link

index failure on 32bit arm #359

Open LocutusOfBorg opened 3 years ago

LocutusOfBorg commented 3 years ago

Hello, I don't know why, but there is a regression in our test in 32bit arm, 5.0.17.15 is fine 5.0.18 is not working as expected.

This is the test

#!/bin/bash
set -e
ret="$(hoogle categoryToCInt)"
test "$ret" = "System.Locale.SetLocale categoryToCInt :: Category -> CInt"

and the failure is:

Removing autopkgtest-satdep (0) ...
autopkgtest [01:19:56]: test setlocale: [-----------------------
hoogle: The Hoogle file /var/lib/hoogle/databases/default.hoo is truncated, probably due to an error during creation.
CallStack (from HasCallStack):
  errorIO, called at src/General/Store.hs:184:13 in hoogle-5.0.18-Js8mCxE9G7N4SmDHTQjt1l:General.Store
CallStack (from HasCallStack):
  errorIO, called at src/General/Util.hs:270:66 in hoogle-5.0.18-Js8mCxE9G7N4SmDHTQjt1l:General.Util
autopkgtest [01:19:57]: test setlocale: -----------------------]
autopkgtest [01:19:57]: test setlocale:  - - - - - - - - - - results - - - - - - - - - -
setlocale            FAIL non-zero exit status 1

Do you have any idea? https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=971832

ndmitchell commented 3 years ago

I would be surprised if it wasn't related to https://github.com/ndmitchell/hoogle/pull/339 as that is the only thing that has changed in the database, and its all about changing word/bit sizes. The other possibility is that the database has overflowed some size, and we used to use Word, but I can't see any evidence of that in the code.

LocutusOfBorg commented 3 years ago

ok but why is it passing on i386? this is what I don't get... do you want me to revert that pull and try again?

ndmitchell commented 3 years ago

Do you know for sure it works on 32 bit? If you could bisect on ARM that would be ideal, reverting that patch is my guess for where the bisect will land. If you could get a copy of the .hoo file mentioned that would let me determine whether writing or reading of the database is messed up.

LocutusOfBorg commented 3 years ago

Hello, sorry for the late answer, but the Debian testing infrastructure on i386 was still under development. Now the issue is tracked also at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=971832

If you look at here https://packages.qa.debian.org/h/haskell-hoogle.html You can see logs for each testing architecture, e.g. https://ci.debian.net/packages/h/haskell-hoogle/testing/amd64 https://ci.debian.net/packages/h/haskell-hoogle/testing/armhf and https://ci.debian.net/packages/h/haskell-hoogle/testing/i386

I requested some tests right now to refresh the logs.

https://ci.debian.net/user/locutusofborg@debian.org/jobs

default.zip

LocutusOfBorg commented 3 years ago

Same with 5.0.18.1 https://ci.debian.net/data/autopkgtest/testing/armhf/h/haskell-hoogle/12253024/log.gz

LocutusOfBorg commented 3 years ago

Hello, reverting #339 "fixes" the issue for me https://autopkgtest.ubuntu.com/results/autopkgtest-impish-costamagnagianfranco-locutusofborg-ppa/impish/armhf/h/haskell-hoogle/20210511_133741_fad60@/log.gz

this is the diff

--- haskell-hoogle-5.0.17.15+dfsg1/debian/patches/339-revert.patch  1970-01-01 00:00:00.000000000 +0000
+++ haskell-hoogle-5.0.18.1+dfsg1/debian/patches/339-revert.patch   2021-05-11 07:36:59.000000000 +0000
@@ -0,0 +1,65 @@
+Description: Revert https://github.com/ndmitchell/hoogle/pull/339/files to possibly fix https://github.com/ndmitchell/hoogle/issues/359
+
+--- haskell-hoogle-5.0.18.1+dfsg1.orig/src/Output/Types.hs
++++ haskell-hoogle-5.0.18.1+dfsg1/src/Output/Types.hs
+@@ -128,15 +128,11 @@ searchFingerprintsDebug store query answ
+ 
+ data TypesNames a where TypesNames :: TypesNames (BStr0, V.Vector Name) deriving Typeable
+ 
+--- At around 7000 packages, Word16 becomes insufficient
+--- because there are more than 2^16 Names, so we use Word32.
+-type NameWord = Word32
+-
+ -- Must be a unique Name per String.
+ -- First 0-99 are variables, rest are constructors.
+ -- More popular type constructors have higher numbers.
+ -- There are currently about 14K names, so about 25% of the bit patterns are taken
+-newtype Name = Name NameWord deriving (Eq,Ord,Show,Data,Typeable,Storable,Binary)
++newtype Name = Name Word16 deriving (Eq,Ord,Show,Data,Typeable,Storable,Binary)
+ 
+ name0 = Name 0 -- use to represent _
+ 
+@@ -154,7 +150,7 @@ prettyName x@(Name i)
+ -- | Give a name a popularity, where 0 is least popular, 1 is most popular
+ popularityName :: Name -> Double
+ popularityName (Name n) | isVar $ Name n = error "Can't call popularityName on a Var"
+-                        | otherwise = fromIntegral (n - 100) / fromIntegral (maxBound - 100 :: NameWord)
++                        | otherwise = fromIntegral (n - 100) / fromIntegral (maxBound - 100 :: Word16)
+ 
+ newtype Names = Names {lookupName :: Str -> Maybe Name}
+ 
+@@ -194,10 +190,10 @@ spreadNames [] = []
+ spreadNames (sortOn (negate . snd) -> xs@((_,limit):_)) = check $ f (99 + fromIntegral (length xs)) maxBound xs
+     where
+         check xs | all (isCon . snd) xs && length (nubOrd $ map snd xs) == length xs = xs
+-                 | otherwise = error $ "Invalid spreadNames, length=" ++ show (length xs)
++                 | otherwise = error "Invalid spreadNames"
+ 
+         -- I can only assign values between mn and mx inclusive
+-        f :: NameWord -> NameWord -> [(a, Int)] -> [(a, Name)]
++        f :: Word16 -> Word16 -> [(a, Int)] -> [(a, Name)]
+         f !mn !mx [] = []
+         f mn mx ((a,i):xs) = (a, Name real) : f (mn-1) (real-1) xs
+             where real = fromIntegral $ max mn $ min mx ideal
+@@ -266,16 +262,14 @@ fpRaresFold :: (b -> b -> b) -> (Name ->
+ fpRaresFold g f Fingerprint{..} = f fpRare1 `g` f fpRare2 `g` f fpRare3
+ 
+ instance Storable Fingerprint where
+-    sizeOf _ = 3*sizeOf name0 + 2
++    sizeOf _ = 64
+     alignment _ = 4
+     peekByteOff ptr i = Fingerprint
+-        <$> peekByteOff ptr (i+0) <*> peekByteOff ptr (i+1*w) <*> peekByteOff ptr (i+2*w)
+-        <*> peekByteOff ptr (i+3*w) <*> peekByteOff ptr (i+3*w + 1)
+-        where w = sizeOf name0
++        <$> peekByteOff ptr (i+0) <*> peekByteOff ptr (i+2) <*> peekByteOff ptr (i+4)
++        <*> peekByteOff ptr (i+6) <*> peekByteOff ptr (i+7)
+     pokeByteOff ptr i Fingerprint{..} = do
+-        pokeByteOff ptr (i+0) fpRare1 >> pokeByteOff ptr (i+1*w) fpRare2 >> pokeByteOff ptr (i+2*w) fpRare3
+-        pokeByteOff ptr (i+3*w) fpArity >> pokeByteOff ptr (i+3*w + 1) fpTerms
+-        where w = sizeOf name0
++        pokeByteOff ptr (i+0) fpRare1 >> pokeByteOff ptr (i+2) fpRare2 >> pokeByteOff ptr (i+4) fpRare3
++        pokeByteOff ptr (i+6) fpArity >> pokeByteOff ptr (i+7) fpTerms
+ 
+ toFingerprint :: Sig Name -> Fingerprint
+ toFingerprint sig = Fingerprint{..}
ndmitchell commented 3 years ago

Thanks for the info. I don't immediately see the problem, so someone with access to a 32bit build will need to debug it. Nothing in this module uses an unsized Word, so I think it should work equivalently, but it's easy enough to make a mistake.

I consider this bug fairly low priority, as I'm not totally convinced that Hoogle will work in a meaningful way with < 4Gb of RAM, especially for database construction, so use 64 bit machines would be my standard advice. That said, I'd happily take a patch.