#61 Sqrat now has problem binding const char * to string in Squirrel

None
closed
nobody
None
5
2014-08-08
2014-03-03
Andy Tai
No

With the last commit

commit d1cb7d6e4d4807fe4c5699e149b04f76d2b39722
Author: Wizzard033 bhaffen97@gmail.com
Date: Sat Mar 1 13:23:03 2014 -0800

Fixed variable get accessors to always return a reference if they can (thanks Aerizeon)

NOTE: When making a custom type that is not referencable, you now MUST use SCRAT_MAKE_NONREFERENCABLE( type
see changes to sqratTypes.h for more information

now Sqrat has problem binding a const char * field to Squirrel, as demonstrated by this unit test

class EmployeeName {
public:
const SQChar * firstName;
const SQChar * lastName;
};

TEST_F(SqratTest, SimpleMembers) {
DefaultVM::Set(vm);
Class<EmployeeName> name;
name.Var(_SC("firstName"), &EmployeeName::firstName)
.Var(_SC("lastName"), &EmployeeName::lastName);

RootTable().Bind(_SC("EmployeeName"), name);

Script script;
script.CompileString(_SC(" \
    steve <- EmployeeName(); \
    steve.lastName = \"Jones\"; \
    \
    gTest.EXPECT_STR_EQ(steve.lastName, \"Jones\"); \
    "));
if (Sqrat::Error::Instance().Occurred(vm)) {
    FAIL() << _SC("Compile Failed: ") << Sqrat::Error::Instance().Message(vm);
}

script.Run();
if (Sqrat::Error::Instance().Occurred(vm)) {
    FAIL() << _SC("Run Failed: ") << Sqrat::Error::Instance().Message(vm);
}

}

the above unit test would crash in
script.Run();
(and in this line
steve.lastName = "Jones";
in Squirrel)
and valgrnd would report of invalid memory read

This problem would go away if you revert to the earlier commit b41095bfd19c9823fb183ef7d809c55b85f6957f

I have added the above test case to the Sqrat unit tests

Discussion

  • Andy Tai
    Andy Tai
    2014-03-03

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -48,4 +48,6 @@
     the above unit test would crash and valgrnd would report of invalid memory read
    
    -This problkm would go away if you revert to the earlier commit b41095bfd19c9823fb183ef7d809c55b85f6957f
    +This problem would go away if you revert to the earlier commit b41095bfd19c9823fb183ef7d809c55b85f6957f
    +
    +I have added the above test case to the Sqrat unit tests
    
    • Group: -->
     
  • Andy Tai
    Andy Tai
    2014-03-03

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -45,7 +45,9 @@
    
     }
    
    -the above unit test would crash and valgrnd would report of invalid memory read
    +the above unit test would crash in     
    +script.Run(); 
    +and valgrnd would report of invalid memory read
    
     This problem would go away if you revert to the earlier commit b41095bfd19c9823fb183ef7d809c55b85f6957f
    
     
  • Andy Tai
    Andy Tai
    2014-03-03

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -47,6 +47,9 @@
    
     the above unit test would crash in     
     script.Run(); 
    +(and in this line         
    +steve.lastName = "Jones"; 
    +in Squirrel)
     and valgrnd would report of invalid memory read
    
     
  • Andy Tai
    Andy Tai
    2014-03-03

    Valgrind output on an x86-64 machine running Fedora 20:

    atai@HPQuad build$ valgrind ./class_instances
    ==13397== Memcheck, a memory error detector
    ==13397== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
    ==13397== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
    ==13397== Command: ./class_instances
    ==13397==
    [==========] Running 4 tests from 1 test case.
    [----------] Global test environment set-up.
    [----------] 4 tests from SqratTest
    [ RUN ] SqratTest.SimpleMembers
    [ OK ] SqratTest.SimpleMembers
    [ RUN ] SqratTest.ClassInstances
    ==13397== Invalid read of size 8
    ==13397== at 0x419E32: long long Sqrat::sqDefaultGet<Employee, char="" const*="">(SQVM) (sqratClassType.h:126)
    ==13397== by 0x45A9BA: SQVM::CallNative(SQNativeClosure
    , long long, long long, SQObjectPtr&, bool&) (sqvm.cpp:1169)
    ==13397== by 0x45C2AB: SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) (sqvm.cpp:1541)
    ==13397== by 0x432058: sq_call (sqapi.cpp:1117)
    ==13397== by 0x40E430: Sqrat::sqVarGet(SQVM) (sqratMemberMethods.h:5255)
    ==13397== by 0x45A9BA: SQVM::CallNative(SQNativeClosure
    , long long, long long, SQObjectPtr&, bool&) (sqvm.cpp:1169)
    ==13397== by 0x45C2AB: SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) (sqvm.cpp:1541)
    ==13397== by 0x45B10B: SQVM::FallBackGet(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&) (sqvm.cpp:1288)
    ==13397== by 0x45ACD9: SQVM::Get(SQObjectPtr const&, SQObjectPtr const&, SQObjectPtr&, bool, long long) (sqvm.cpp:1226)
    ==13397== by 0x4569C3: SQVM::Execute(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long, SQVM::ExecutionType) (sqvm.cpp:806)
    ==13397== by 0x45C27A: SQVM::Call(SQObjectPtr&, long long, long long, SQObjectPtr&, unsigned long long) (sqvm.cpp:1537)
    ==13397== by 0x432058: sq_call (sqapi.cpp:1117)
    ==13397== Address 0x10 is not stack'd, malloc'd or (recently) free'd

     
  • Wizzard
    Wizzard
    2014-03-03

    No time to fix this right at this moment, but I went ahead and found out why its breaking

    template<class T>
    inline void PushVarR(HSQUIRRELVM vm, T& value) {
        std::cout << "Type of template argument: " << typeid(T).name() << '\n';
        std::cout << "Type after transform of previous: " << typeid(typename remove_const<typename remove_reference<T>::type>::type).name() << '\n';
        std::cout << "Type of const char*: " << typeid(const char*).name() << '\n';
        std::cout << "Type after transform of previous: " << typeid(typename remove_const<typename remove_reference<const char*>::type>::type).name() << '\n';
        std::cout << "Type needs to be (ie char*): " << typeid(char*).name() << '\n';
        if (is_referencable<typename remove_const<typename remove_reference<T>::type>::type>::value) {
            Var<T&>::push(vm, value); // is true when it should not be
        } else {
            PushVar(vm, value); // const char* should do this instead
        }
    }
    

    Its because remove_const (and maybe remove_reference) aren't working as I intended

     
  • Andy Tai
    Andy Tai
    2014-03-08

    the following fixes the failed test case

    commit 0acb835909f0c95db334ef345a0d1f6231233b3e
    Sqrat Sourceforge Bug 61: possible fix
    specially handling of const T* for remove_const<type>

     
  • Andy Tai
    Andy Tai
    2014-03-08

    • status: open --> closed