andyarvanitis / purescript-native-cpp-ffi

C++ foreign export implementations for the standard library
MIT License
21 stars 8 forks source link

stack overflow with simple code #2

Closed iomeone closed 5 years ago

iomeone commented 5 years ago

hi, another issue, I want to use Data.Char.Unicode's isLetter function, but I found that the call to isLetter causes stack overflow, can you check it . btw: I've set the stack reserve size to 100MB, when the error occured, I checked the memory the process occupied and showed beyond 100MB.

module Main where

import Prelude hiding (between,when)
import Data.Char.Unicode (digitToInt, isDigit, isLetter)
import Effect (Effect)
import Effect.Console (logShow)

main :: Effect Unit
main = do
  logShow (isLetter '4')

this function will be called over and over and finally case stack overflow!

auto bsearch() -> const boxed& {
    static const boxed _ = [](const boxed& a) -> boxed {
        return [=](const boxed& array) -> boxed {
            return [=](const boxed& size) -> boxed {
                return [=](const boxed& compare) -> boxed {
                    boxed::recur go;
                    boxed::recur::weak go_Weak_(go);
                    go = [=](const boxed& i) -> boxed {
                        boxed go = go_Weak_;
                        return [=](const boxed& k) -> boxed {
                            if (unbox<bool>(Data_Ord::greaterThan()(Data_Ord::ordInt())(i)(k))) {
                                return Data_Maybe::Nothing();
                            };
                            if (unbox<bool>(Data_Boolean::otherwise())) {
                                boxed j = Data_Int::floor()(unbox<double>(Data_Int::toNumber()(unbox<int>(i) + unbox<int>(k))) / 2.0);
                                boxed b = [=](const boxed& dictPartial) -> boxed {
                                    return Data_Array::unsafeIndex()(dictPartial)(array)(j);
                                }(undefined);
                                boxed v = compare(a)(b);
                                if (unbox<dict_t>(v).contains("EQ")) {
                                    return Data_Maybe::Just()(b);
                                };
                                if (unbox<dict_t>(v).contains("GT")) {
                                    return go(unbox<int>(j) + 1)(k);
                                };
                                return go(i)(unbox<int>(j) - 1);
                            };
                            THROW_("PatternMatchFailure: ""Failed pattern match at Data.Char.Unicode.Internal (line 4783, column 5 - line 4789, column 49): ");
                        };
                    };
                    return go(0)(size);
                };
            };
        };
    };
    return _;
};

corresponding purs code is

-- The third argument is a  size.  It says to only look at this many elements
-- in the array.  There are times when we don't want to look at ALL the
-- elements, but only a subset.
bsearch :: forall a . a -> Array a -> Int -> (a -> a -> Ordering) -> Maybe a
bsearch a array size compare = go 0 size
    where
    go i k | i > k = Nothing
           | otherwise = let j = floor (toNumber (i + k) / 2.0)
                             b = unsafePartial (Array.unsafeIndex array j)
                         in case compare a b of
                              EQ -> Just b
                              GT -> go (j + 1) k
                              _  -> go i (j - 1)
iomeone commented 5 years ago

I've tried to increase the stack size to 5GB, and after a while, still overflow :(

iomeone commented 5 years ago

boxed j = Data_Int::floor()(unbox<double>(Data_Int::toNumber()(unbox<int>(i) + unbox<int>(k))) / 2.0);

I think this line cause the bug! I set an breakpoint in this line , i is 0, k is 2783 . I step into the ffi of floor, it returns an int of 1391, which is also right, but in the very last step (return from floor) , it calls

auto semigroupoidFn() -> boxed {
    return Control_Semigroupoid::Semigroupoid()([=](const boxed& f) -> boxed {
        return [=](const boxed& g) -> boxed {
            return [=](const boxed& x) -> boxed {
                return f(g(x));
            };
        };
    });
};

and then j value changed to 0, why??? what's the purpose of semigroupoidFn?

iomeone commented 5 years ago

so the problem has simplified to

    boxed b_testInt = Data_Int::floor()(2.5);
    int testInt = unbox<int>(b_testInt);

the testInt will be 0 . I can not get it right , please help!!

iomeone commented 5 years ago
auto unsafeClamp() -> const boxed& {
    static const boxed _ = [](const boxed& x) -> boxed {
        if (unbox<bool>(Data_Eq::eq()(Data_Eq::eqNumber())(x)(Global::infinity()))) {
            return 0;
        };
        if (unbox<bool>(Data_Eq::eq()(Data_Eq::eqNumber())(x)(-unbox<double>(Global::infinity())))) {
            return 0;
        };
        if (unbox<bool>(Data_Ord::greaterThanOrEq()(Data_Ord::ordNumber())(x)(Data_Int::toNumber()(Data_Bounded::top()(Data_Bounded::boundedInt()))))) {
            return Data_Bounded::top()(Data_Bounded::boundedInt());
        };
        if (unbox<bool>(Data_Ord::lessThanOrEq()(Data_Ord::ordNumber())(x)(Data_Int::toNumber()(Data_Bounded::bottom()(Data_Bounded::boundedInt()))))) {
            return Data_Bounded::bottom()(Data_Bounded::boundedInt());
        };
        return Data_Maybe::fromMaybe()(0)(Data_Int::fromNumber()(x));
    };
    return _;
};

auto floor() -> boxed {
    return Control_Semigroupoid::compose()(Control_Semigroupoid::semigroupoidFn())(Data_Int::unsafeClamp())(Math::floor());
};

In Data_Int.cpp , the floor() function ,
First: the Math::floor() return a box Second: call unsafeClamp, the arg is an box , but the code if (unbox<bool>(Data_Eq::eq()(Data_Eq::eqNumber())(x)(Global::infinity()))) {

eqNumber())(x) will unbox an double, which will be an random value !!!

call stack is :


auto eqNumber() -> boxed {
    return Data_Eq::Eq()(Data_Eq::eqNumberImpl());
};
auto eqNumberImpl() -> const boxed& { static const boxed _ = foreign().at("eqNumberImpl"); return _; };

exports["eqNumberImpl"] = [](const boxed& x_) -> boxed {
    const auto x = unbox<double>(x_);
    return [=](const boxed& y_) -> boxed {
        const auto y = unbox<double>(y_);
        return std::abs(x-y) <= std::numeric_limits<double>::epsilon() * std::abs(x+y) ||
               std::abs(x-y) < std::numeric_limits<double>::min();
    };
};

look ! the first line of code (unbox(x_);) unbox an double ?????? Is this a bug??

update: this is not an bug , because the ffi of exports["floor"] and exports["round"] should return double .

iomeone commented 5 years ago

bug found: this funciton in ffi ,

exports["eqNumberImpl"] = [](const boxed& x_) -> boxed {
    const auto x = unbox<double>(x_);
    return [=](const boxed& y_) -> boxed {
        const auto y = unbox<double>(y_);
        return std::abs(x-y) <= std::numeric_limits<double>::epsilon() * std::abs(x+y) ||
               std::abs(x-y) < std::numeric_limits<double>::min();
    };
};

for example ,x = 1.0, y = std::numeric_limits<double>::infinity() the function will say x and y are equal !

andyarvanitis commented 5 years ago

Good catch. I need to add something like this before the check:

if (x == std::numeric_limits<double>::infinity() ||
    y == std::numeric_limits<double>::infinity()) {
   return (x == y);
}
...
iomeone commented 5 years ago

I am dout 1, when x y are both infinity() , is they equal?

2,Shoud we handle the Nan situation ?

Please close this issue , when you correct the bug, my bug is solved , and I'm busy with testing lots of purescript package with purescript-native

andyarvanitis commented 5 years ago

Well, that was just an example, not necessarily the exact (and tested) solution. Glad you are able to proceed, though, and thanks for the report.