v3.13 -> RapidJSON Crashes in Release mode on Android

[ermanhaskan]

NDK: r12b
API-LEVEL: 11

Rapidjson's memory allocation functions crash every time when compiled in release mode. Fix is to use custom types instead of rapidjson::Document and rapidjson::Value.

typedef rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> RapidJsonDocument;
typedef rapidjson::GenericValue<rapidjson::UTF8<>, rapidjson::CrtAllocator> RapidJsonValue;

[dumganhar]

How about changing API-LEVEL to 10 and make sure API 10 is installed?

[ermanhaskan]

Haven't tried api level 10 but it was working without any problems in previous cocos versions on API-LEVEL 11. I think it has something to do with gcc changing in 3.13.

[minggo]

@ermanhaskan we just move back to gcc, only 3.12 uses clang. And how to reproduce it?

[dumganhar]

rapidjson is used in CocosStudio editor reader. But currently, we have removed the test case in cpp-tests. Could @ermanhaskan add a test case to cpp-tests -> UnitTest for this issue?

[pbs0512]

NDK: r12b
API : 23
engine : cocos2d-x 3.13 Lua
I have crashes when running in debug mode

The first run was fine. Since then crash

Fix :
cocos2d-x/tools/simulator/libsimulator/lib/runtime/FileServer.cpp
`void FileServer::readResFileFinfo()
{
std::string filecfg = _writePath + "/fileinfo_debug.json";
FILE * pFile = fopen (filecfg.c_str() , "r");
if(pFile)
{
rapidjson::FileStream inputStream(pFile);
_filecfgjson.ParseStream<0>(inputStream);
fclose(pFile);
}
if(! _filecfgjson.IsObject()){
_filecfgjson.SetObject();
}

////save file info to disk every five second 
//Director::getInstance()->getScheduler()->schedule([&](float){
//    rapidjson::StringBuffer buffer;
//    rapidjson::Writer< rapidjson::StringBuffer > writer(buffer);
//    _filecfgjson.Accept(writer);
//    const char* str = buffer.GetString();
//    std::string filecfg = _writePath + "/fileinfo_debug.json";
//    FILE * pFile = fopen(filecfg.c_str(), "w");
//    if (!pFile) return ;
//    fwrite(str, sizeof(char), strlen(str), pFile);
//    fclose(pFile);
//},this, 5.0f, false, "fileinfo");

}`

[minggo]

@Finebe did you use simulator to run lua scripts?

[dumganhar]

@ermanhaskan , I reproduced this issue now. Do you know the reason why it crashes in release mode?

[ermanhaskan]

@dumganhar it's about memory allocation but have no idea why.

[dumganhar]

UPDATED:

Although I still don't know the reason, I just added an unused line to printf something at the following function:

    GenericDocument(Allocator* allocator = 0, size_t stackCapacity = kDefaultStackCapacity, StackAllocator* stackAllocator = 0) : 
        allocator_(allocator), ownAllocator_(0), stack_(stackAllocator, stackCapacity), parseResult_()
    {
        if (!allocator_) {
            ownAllocator_ = allocator_ = RAPIDJSON_NEW(Allocator());
            printf("allocator: %p", allocator); // ADDED THIS LINE
        }
    }

And everything works fine.

[dumganhar]

@miloyip, do you have any idea about my above comment?

[dumganhar]

@ermanhaskan， is older versions of cocos2d-x also trigger this issue? Or it's just happens in 3.13?

[dumganhar]

I found that if we add LOCAL_ARM_MODE := arm to Android.mk, rapidjson will crash.
It's probably an issue about arm mode optimization.

[ermanhaskan]

@dumganhar this was not happening before 3.13 and I am using LOCAL_ARM_MODE := arm for years.

[dumganhar]

hello-jni-rapidjson-crash.zip

@ermanhaskan , I reproduced this issue with the simplest code by modifying hellojni.
And I confirm that it's the issue of using LOCAL_ARM_MODE := arm & rapidjson.
It's probably that rapidjson uses some features those aren't supported in gcc 4.9.
Or it's probably a bug of gcc 4.9.

BTW, if you use clang to compile, every thing will work fine.

[ermanhaskan]

@dumganhar how can I switch to clang?

[dumganhar]

@ermanhaskan,

cocos2d-x/tools/cocos2d-console/plugins/plugin_compile/build_android.py

    def get_toolchain_version(self, ndk_root, compile_obj):
        return '4.9' # return 'clang' to switch to 'clang' to compile

But currently, clang with gnustl_static stl library isn't stable enough. So we keep on using gcc + gnustl_static.
However some developers use clang for a long time and they don't find any issues with clang + c++_static in cocos2d-x.

So, you could test by modifying cocos2d-console to use clang.

And modify Application.mk to use c++_static.

APP_STL := gnustl_static

-->

APP_STL := c++_static

[miloyip]

I have tried different configurations, and found that gcc 4.9.0 arm mode release configuration (-O2) causes crash. Either changing it to clang, debug configuration, or thumb instruction set do not have the problem.

The current minimal reproducible code is simply:

Document d;
d.Parse("[1]");

This will cause crash due to incorrect instruction emitted in SetNull(). If we turn off optimization in SetNull(), then crash will not happened. But adding SetObject() (or similar functions) can cause crash as well.

So that, a workaround is to reduce optimization level to -O1 when including document.h. This can be done by following code:

#include "rapidjson/rapidjson.h"

// Bug in gcc 4.9.0 on ARM with ARM instruction set, -O2 causes the bug
// This gcc version cannot use #pragma GCC push_options/pop_options as well. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59884
#if defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0) && defined(__arm__)
    #pragma GCC optimize ("O1")
#endif    
#include "rapidjson/document.h"
#if defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0) && defined(__arm__)
    #pragma GCC reset_options
#endif    

This workaround only fixes on gcc 4.9.x and ARM.

[patriciog]

Hi @dumganhar, @minggo:

Our game is crashing when ccs.load(...) is loaded in release mode, because it use rapidjson too.

Will @miloyip's workaround be the final solution in cocos2d-x 3.14 or are you working on this issue?

Thanks and best,

[patriciog]

Thanks @miloyip!

Your workaround works fine.

[minggo]

@miloyip can the compiling option be included in rapidjson/rapidjson.h?

[patriciog]

Hi:

I've added this code in all CocosStudio classes where document.h is included:

// #include "json/document.h"
// Bug in gcc 4.9.0 on ARM with ARM instruction set, -O2 causes the bug
// This gcc version cannot use #pragma GCC push_options/pop_options as well. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59884
#include "json/rapidjson.h"
#if defined(__arm__) && defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0)
    #pragma GCC optimize ("O1")
#endif    
#include "json/document.h"
#if defined(__arm__) && defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0)
    #pragma GCC reset_options
#endif

It's working.

[minggo]

@patriciog thanks for the information, but i think it is better to add it in document.h if possible. Or it is inconvenient to add the codes in all codes includes document.h.

[miloyip]

I do not want to do such workaround in the master. This seem happen in some versions of compilers on some platforms, such kind of patching may make the code unclean.

I suggest that, you may create a own wrapper header, which contains such patching and include the real header. And then replacing the existing usage of document.h with the wrapper header.

[dumganhar]

@minggo ,PR was submitted here (cocos2d/cocos2d-x-3rd-party-libs-bin#254)

[minggo]

@dumganhar why rename it to document-real.h? It will break compatibility. And i think it is better to add a header file in cocos2d-x without modify the lib, or it will hard to update rapid json in future.

[dumganhar]

@minggo, since @miloyip suggest don't modify rapidjson source code to fix this issue, I simply added a new file document.h as a wrapper file in which invokes the real document.h.

[minggo]

@dumganhar doing like this will be convenient for developers, but it is hard to update rapid json in future. document.h will be replaced when upgrading rapid json, we can not remember to do the special operation when upgrading.

What i mean doing the wrapper in cocos2d-x is that:

• adding a documentwrapper.h in cocos2d-x, the contents of it is the same as your current document.h
• the source codes includes documentwrapper.h instead of document.h

This method is good for upgrading, but should let developers know the new usage.

[dumganhar]

Okay, so every developer should know to include json/document-wrapper.h rather than the standard rapidjson header file json/document.h. We have to teach every developer not to use json/document.h. If a developer does, he/she probably create an issue saying that her/his game crash, we have to explain that hey guy, don't use the document.h in rapidjson, please use my wrapper. Yes, we could announce in release note for this, but there will still be some developers who don't read the release note and report this issue.

Therefore,

• Keep original json/document.h + add json/document-wrapper.h: Teach every developer to use json/document-wrapper.h
• Rename json/document.h to json/document-real.h + make new json/document.h as wrapper file: Only the 3rd library maintainer should notice this, we could probably add a README in cocos2d-x-3rd-party-libs-bin repo.

IMO, we should make developer use rapidjson as before, i mean, use it like how rapidjson official teach.

Yes, if @miloyip could apply the workaround into upstream, that will be best for us.

I closed my PR for 3rd party library repo temporarily.

[patriciog]

Hi @dumganhar:

I've created a wrapper following your steps for my game and it works:

frameworks\changes\cocos2d-x\external\json\document-wrapper.h:

#pragma once

// Wrapper file for document.h to fix the crash bug (https://github.com/cocos2d/cocos2d-x/issues/16492)

// Bug in gcc 4.9.0 on ARM with ARM instruction set, -O2 causes the bug
// This gcc version cannot use #pragma GCC push_options/pop_options as well. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59884
#include "json/rapidjson.h"
#if defined(__arm__) && defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0)
    #pragma GCC optimize ("O1")
#endif    
#include "document.h"
#if defined(__arm__) && defined(RAPIDJSON_GNUC) && RAPIDJSON_GNUC >= RAPIDJSON_VERSION_CODE(4,9,0) && RAPIDJSON_GNUC < RAPIDJSON_VERSION_CODE(5,0,0)
    #pragma GCC reset_options
#endif

After I've replaced all instances of "#include "json/document-wrapper.h"" by "#include "json/document-wrapper.h"

Thanks and best!

[dumganhar]

@patriciog, thanks for testing. I will submit a Pull Request for this.

[dumganhar]

PULL REQUEST DONE( #16792)

[jarsj]

My crash went away after I changed to using CRT Allocator, like this

// Old Code
rapidjson::Document d;

// New Code
typedef rapidjson::GenericDocument<rapidjson::UTF8<>,rapidjson::CrtAllocator> CRTDocument;
CRTDocument d;

[rraallvv]

After my game runs for the first time I'm getting this:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000122
Stack frame #00  pc 0062d970  /data/app-lib/org.cocos2d-x.mygame-1/libcocos2djs.so: Routine void MarkInternal<JSObject>(JSTracer*, JSObject**) at proxy.cpp:?
Stack frame #01  pc 00635bb8  /data/app-lib/org.cocos2d-x.mygame-1/libcocos2djs.so: Routine js::gc::GCRuntime::markRuntime(JSTracer*, bool) at libgcc2.c:?
Crash dump is completed

Then I'm getting this consistently, again and again.

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 2e62736a
Stack frame #00  pc 00a0c9fe  /data/app-lib/org.cocos2d-x.mygame-1/libcocos2djs.so (rapidjson::Writer<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::WriteString(char const*, unsigned int)+33): Routine rapidjson::Writer<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::WriteString(char const*, unsigned int) at ??:?
Stack frame #01  pc 00a0cbd1  /data/app-lib/org.cocos2d-x.mygame-1/libcocos2djs.so (bool rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Accept<rapidjson::Writer<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>, rapidjson::CrtAllocator>, rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator> >(rapidjson::Writer<rapidjson::GenericStringBuffer<rapidjson::UTF8<char>,+264): Routine $t at proxy.cpp:?
Stack frame #02  pc 010cc69b  /data/app-lib/org.cocos2d-x.mygame-1/libcocos2djs.so: Routine std::_Function_handler<void (float), FileServer::readResFileFinfo()::{lambda(float)#1}>::_M_invoke(std::_Any_data const&, float) at cpu-features.c:?
Crash dump is completed

Are those issues related to this bug?
I'm using Cocos2d-x v3.9

I'd highly appreciate any information.

[dumganhar]

@rraallvv, it is probably the same issue. Please try the fix above.

[rraallvv]

@dumganhar thanks for the info. I'll give it a try.

[georges-berenger]

FWIW, we just ran into the same issue on Windows, with release & debug builds alike, 100% reproducible. Merely switching to CRTDocument magically resolved our issues. We are building for MacOS, Ubuntu, Android and Windows.
It seems to me that this isn't a platform or compiler issue, but rather, a bug in the default MemoryPool allocator that some are running into, probably because of some complex combination of factors that's hard to reproduce unless you're running into it "by chance".

(and one week-end down the drain, hunting down some library bug... :-( )

[miloyip]

@georges-berenger Is it possible to make a minimal reproducible example?

[georges-berenger]

Unfortunately, no. I've spent a nice chunk of time trying to make the issue happen in our unit tests, but I was unable to get anything there.

The issue started happening after we made a very unrelated project change, but then, it would happen 100% of the time on the same very "innocent looking" rapidjson code after making a call to SetString(const char*, size, alloc) and returning the rapidjson::Value.

BTW, thanks @jarsj for the idea of using CrtAllocator. This is a much more reasonable fix than disabling optimizations.

Is there a chance that it could a symbol resolution issue between libraries?