Crash when passing nearly-empty document #301

RSpliet · 2015-04-14T07:28:54Z

RapidJSON (debian, 0.12~git20141031-1) seems to crash when I pass it as a string the simple document "{ }". Expected is either a graceful failure in hasParseError() or a return false on subsequent hasMember() requests. Instead, the following occurs:

Program received signal SIGSEGV, Segmentation fault.
1551            data_.o.members = (Member*)allocator.Malloc(count * sizeof(Member));
(gdb) bt
#0  SetObjectRaw (allocator=..., count=0, members=0x1edc20, this=0x1edc08) at /usr/include/rapidjson/document.h:1551
#1  EndObject (memberCount=0, this=<optimized out>) at /usr/include/rapidjson/document.h:1852
#2  rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseObject<0u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rapidjson::GenericDocument<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>, rapidjson::CrtAllocator> > (this=<optimized out>, is=..., 
    handler=...) at /usr/include/rapidjson/reader.h:467
#3  0x00034662 in Parse<0u, rapidjson::GenericStringStream<rapidjson::UTF8<> >, rapidjson::GenericDocument<rapidjson::UTF8<> > > (handler=..., is=..., this=0xbefff4c8)
    at /usr/include/rapidjson/reader.h:397
#4  ParseStream<0u, rapidjson::UTF8<>, rapidjson::GenericStringStream<rapidjson::UTF8<> > > (is=..., this=0xbefff500) at /usr/include/rapidjson/document.h:1698
#5  Parse<0u, rapidjson::UTF8<> > (str=<optimized out>, this=0xbefff500) at /usr/include/rapidjson/document.h:1774
#6  Parse<0u> (str=<optimized out>, this=0xbefff500) at /usr/include/rapidjson/document.h:1783
#7  Parse (str=<optimized out>, this=0xbefff500) at /usr/include/rapidjson/document.h:1790
#8  Settings::fillByJSON (this=this@entry=0x1edbc0, json="{ }") at /Embedded/src/Util/Settings.cpp:75
#9  0x0003dd96 in ArcusView::doSettings (this=this@entry=0x97b80, msg=msg@entry=0xb3b00518) at /Embedded/src/View/ArcusView.cpp:56
#10 0x0002c17a in ArcusView::main (this=this@entry=0x97b80) at /Embedded/src/View/ArcusView.cpp:364
#11 0x0002c8a6 in main (argc=<optimized out>, argv=<optimized out>) at /Embedded/src/main.cpp:35

Although I hardly know the code, I suspect this might have something to do with malloc() being called with size 0. But I'll leave that bit to the experts!
Could you look into this please?

The text was updated successfully, but these errors were encountered:

miloyip · 2015-04-14T09:43:57Z

Were you able to build and run the unit tests?
And also, I am not sure about the version you are using. Can you try the https://github.com/miloyip/rapidjson/releases/tag/v1.0-beta ?

pah · 2015-04-14T17:39:22Z

I can't confirm this on neither the Debian package nor the current HEAD (or 1.0-beta).
Usually, it is well-defined to call malloc(0).

Which platform/compiler are you using?
Can you provide a reduced example with the calling code to reproduce the problem?
Do you use a custom allocator?

According to the C/C++ standards, calling `memcpy(NULL, NULL, 0)` is undefined behaviour. Recent GCC versions may rely on this by optimizing NULL pointer checks more aggressively, see [1]. This patch tries to avoid calling std::memcpy with zero elements. As a side effect, explicitly return NULL when requesting an empty block from MemoryPoolAllocator::Malloc. This may be related to Tencent#301. [1] https://gcc.gnu.org/gcc-4.9/porting_to.html

pah · 2015-04-14T19:10:51Z

I've pushed a potential fix to my fix/strict-memcpy branch, which avoids calling std::memcpy with NULL pointer arguments. Can you check, if this fixes the segfault for you?

miloyip · 2015-04-15T03:17:59Z

Thanks @pah. This potential fix seems make sense. If the standard says it is undefined, we shall prevent this.
I hope @RSpliet can test this. But even this is not related, I think this fix should be merged to master.

RSpliet · 2015-04-15T09:34:33Z

First of all thank you for your quick replies. TL;DR: patch fixes issues, but program still segfaults when trying a "real" JSON file.

@miloyip I'm afraid unit tests don't build due to (many!) linker errors. The debian package doesn't ship with them, so unless I find the time to investigate this issue I'm afraid we'd have to do without unit test results.
@pah Current head doesn't behave differently. I'm using GCC on ARMv7 (Cortex A7, hard float). No custom allocator was defined. Using the same code on x86_64 works as expected.
My simplified test-program didn't crash, hence I'm not posting one.
Your patch fixes my test program on ARM (with an empty JSON file "{ }"), so hereby you have my tested-by.

However, there is a new issue when testing a bigger file. See the following backtrace:

Program received signal SIGSEGV, Segmentation fault.
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>::Malloc (this=0x0, size=15) at /usr/local/include/rapidjson/allocators.h:167
167         if (chunkHead_ == 0 || chunkHead_->size + size > chunkHead_->capacity)
(gdb) bt
#0  rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>::Malloc (this=0x0, size=15) at /usr/local/include/rapidjson/allocators.h:167
#1  0x0003c3b2 in SetStringRaw (allocator=..., s=..., this=<optimized out>) at /usr/local/include/rapidjson/document.h:1625
#2  GenericValue (allocator=..., length=<optimized out>, s=<optimized out>, this=<optimized out>) at /usr/local/include/rapidjson/document.h:544
#3  String (copy=<optimized out>, length=<optimized out>, str=<optimized out>, this=<optimized out>) at /usr/local/include/rapidjson/document.h:1881
#4  rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseString<0u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rapidjson::GenericDocument<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>, rapidjson::CrtAllocator> > (this=<optimized out>, is=..., 
    handler=..., isKey=<optimized out>) at /usr/local/include/rapidjson/reader.h:657
#5  0x0003c74c in rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseObject<0u, rapidjson::GenericStringStream<rapidjson::UTF8<char> >, rapidjson::GenericDocument<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator>, rapidjson::CrtAllocator> > (this=0xbefff4c0, 
    is=..., handler=...) at /usr/local/include/rapidjson/reader.h:497
#6  0x00038fee in Parse<0u, rapidjson::GenericStringStream<rapidjson::UTF8<> >, rapidjson::GenericDocument<rapidjson::UTF8<> > > (handler=..., is=..., this=0xbefff4c0)
    at /usr/local/include/rapidjson/reader.h:408
#7  ParseStream<0u, rapidjson::UTF8<>, rapidjson::GenericStringStream<rapidjson::UTF8<> > > (is=..., this=0xbefff4f8) at /usr/local/include/rapidjson/document.h:1750
#8  Parse<0u, rapidjson::UTF8<> > (str=<optimized out>, this=0xbefff4f8) at /usr/local/include/rapidjson/document.h:1815
#9  Parse<0u> (str=<optimized out>, this=0xbefff4f8) at /usr/local/include/rapidjson/document.h:1824
#10 Parse (str=<optimized out>, this=0xbefff4f8) at /usr/local/include/rapidjson/document.h:1831
#11 Settings::fillByJSON (this=this@entry=0xaab90, 
    json="{\n    \"name\": \"XXXXXXXXXX\",\n    \"platform\": \"XXXXXXXXXX\",\n    \"required_plugins\": [\n        \"Console Logger\",\n        \"XXXXXXXXXBackend\"\n    ],\n    \"machine_settings\": {\n        \"machine_wid"...) at /Embedded/src/Util/Settings.cpp:75
#12 0x00042a1e in ArcusView::doSettings (this=this@entry=0xa0de0, msg=msg@entry=0xb4203110) at /Embedded/src/View/ArcusView.cpp:56
#13 0x000305ea in ArcusView::main (this=this@entry=0xa0de0) at /Embedded/src/View/ArcusView.cpp:462
#14 0x00030e16 in main (argc=<optimized out>, argv=<optimized out>) at /Embedded/src/main.cpp:35

Valgrind confirms this segfault is a nullptr-exception:

==3096==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Is there anything else I can provide to help understand this issue better?

pah · 2015-04-15T16:35:42Z

The second report seems to be similar to #200.

It may be caused by a failing allocation within the MemoryPoolAllocator on ARM/Android(?). Maybe something else. But it's hard to say without debugging it further.

Maybe you need to use the CrtAllocator as well?

RSpliet · 2015-04-15T20:39:13Z

I shall look into using the CrtAllocator tomorrow (CEST) if it doesn't already, but to answer your question: The ARM platform is running a standard, albeit slim, Debian installation with an upstream Linux kernel 4.0rc4 or rc5. To clarify: there is no Android on this system, just Linux.

miloyip · 2015-04-16T02:00:29Z

I try to analysis the provided core dump.

Accessing this->chunkHead_ fails, due to this == 0
That this is actually GenericDocument<..>::allocator_.
By default, GenericDocument<..>::allocator_ is initialized in constructor, using RAPIDJSON_NEW(Allocator()).
The default macro definition for RAPIDJSON_NEW(x) is new x.

If you can able to gdb, try to trace into the constructor and see if the new operator returns 0.

And the empty JSON object {} case does not fail because it does not need dynamic allocation. You may also run a more simply case to verify this:

Document d;
d.SetString("hello", d.GetAllocator());

RSpliet · 2015-04-16T10:11:46Z

I can confirm that in my test program, the new operator returns 0. I don't completely graps what goes wrong though, when executing the following code-snippet between creation of the object and calling the parse method:

        rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
        printf( "allocator:::%p\n", &allocator );

I get

allocator:::0x1fbf68
Aborted

The test program you wrote executes fine, which points in the direction of... a compiler issue?

pah · 2015-04-16T14:31:47Z

As mentioned above, the symptoms are exactly the same as in #200.
No clue, what's happening, though.

miloyip · 2015-04-16T15:42:16Z

In #200, the reporter solved the issue by CrtAllocator. I am still suspecting there is some issues with MemoryPoolAllocator which caused the problem on ARM.

Apart from @pah 's pull request about std::memcpy(), I can just think of memory alignment issue. The default alignment is 4 bytes, which is enforced in RAPIDJSON_ALIGN. I think you may try to make it as 8 or 16:

#define RAPIDJSON_ALIGN(x) ((x + 3u) & ~3u) // 4
#define RAPIDJSON_ALIGN(x) ((x + 7u) & ~7u) // 8
#define RAPIDJSON_ALIGN(x) ((x + 15u) & ~15u) // 16

Try to define this before including any rapidjson headers.

BTW, this is just a guess.

Another thing that may try, is to redefine new and delete. This can be done by macro RAPIDJSON_NEW(x) and RAPIDJSON_DELETE as well. You may define it as malloc() and then do a placement new. Possible to assert or print something when malloc() fails.

miloyip · 2015-04-16T15:46:59Z

I just found out this issue may be related. Redefining RAPIDJSON_ALIGN as above may solve the problem, rather than adding padding.

According to the C/C++ standards, calling `memcpy(NULL, NULL, 0)` is undefined behaviour. Recent GCC versions may rely on this by optimizing NULL pointer checks more aggressively, see [1]. This patch tries to avoid calling std::memcpy with zero elements. As a side effect, explicitly return NULL when requesting an empty block from MemoryPoolAllocator::Malloc. This may be related to Tencent#301. [1] https://gcc.gnu.org/gcc-4.9/porting_to.html

RSpliet · 2015-04-17T08:35:32Z

Alignment to 4-bytes should be sufficient on ARMv7; the architecture does not specify stricter alignment requirements. Using CrtAllocator does solve the issues, pointing in the direction of the MemoryPoolAllocator.
I suggest to use CrtAllocator by default on ARM for both GenericDocument and GenericValue. Furthermore, and this is not to sound condescending but rather to be helpful, I would be interested to learn what the performance impact of such a move on other platforms would have. My experience is that the default memory allocator on at least Linux is already using memory pools to avoid kernel roundtrips for every allocation in addition to not using the expensive and exhaustive mmap method for smaller allocations. With this paper in mind, benchmarks could give incentive to simplify the current code-base as opposed to spending a lot of time trying to make the MemoryPoolAllocator suitable for ARM.

Either way, I hope this feedback is sufficient to help you get rapidjson running on ARM! Thanks for your time and effort to help me debug.

miloyip · 2015-04-17T09:46:42Z

The MemoryPoolAllocator is actually not really a pool (sorry for bad naming). It is actually a stack-like allocator without deallocation. The advantages include fast and no memory overhead. You may try experimenting this with nativejson-benchmark, which can measure both time and space performance.

I am still interested to solve the issue with MemoryPoolAllocator in ARMv7 platform. We have some projects using RapidJSON that runs on iOS/Android without problem.

Have you tried changing the alignment settings as above? Hopefully if you have time, you may help us to discover the real reason of failing. Thank you.

miloyip · 2015-04-21T05:14:07Z

Keep a note here before closing.

This issue is not fully resolved.
The reason for crashing is still unknown.
The workaround is to use CrtAllocator instead of MemoryPoolAllocator.
Since I am unable to access the platform, hopefully someone with this problem in the past, or in the future, can dig into the problem and report to us.

KumarShrawan · 2015-09-14T05:47:23Z

I was also getting segfault when running my program after compiling with-DCMAKE_BUILD_TYPE=Release, for an ARMv7-A Cortex A9 with GCC 4.9.2-10.
I was able to fix this by replacing "typedef GenericDocument<UTF8<> > Document" by
"typedef rapidjson::GenericDocumentrapidjson::UTF8<,rapidjson::CrtAllocator> Document;" as suggested by Pah.

However, would like to understand the root cause and get the proper solution. if some one gets it let us know as well.

miloyip · 2016-10-01T09:30:14Z

I am not sure if this uses the same configuration as #758, but you may try if that workaround works.

miloyip added the bug label Apr 15, 2015

miloyip added this to the v1.0 RC milestone Apr 15, 2015

pah mentioned this issue Apr 15, 2015

Avoid calling memcpy with NULL pointers #305

Merged

miloyip closed this as completed Apr 21, 2015

pah mentioned this issue Jul 20, 2015

Crash in Release mode on armv7 system #388

Closed

danielmewes mentioned this issue Oct 5, 2015

Investigate RapidJSON MemoryPoolAllocator issue on ARM rethinkdb/rethinkdb#4920

Open

kkaefer mentioned this issue Dec 14, 2015

Switch RapidJSON to CrtAllocator mapbox/mapbox-gl-native#3290

Merged

rh101 mentioned this issue Jul 22, 2024

Two rapidjson with different version cause unexpected crash problem on apple family platforms axmolengine/axmol#2048

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crash when passing nearly-empty document #301

Crash when passing nearly-empty document #301

RSpliet commented Apr 14, 2015

miloyip commented Apr 14, 2015

pah commented Apr 14, 2015

pah commented Apr 14, 2015

miloyip commented Apr 15, 2015

RSpliet commented Apr 15, 2015

pah commented Apr 15, 2015

RSpliet commented Apr 15, 2015

miloyip commented Apr 16, 2015

RSpliet commented Apr 16, 2015

pah commented Apr 16, 2015

miloyip commented Apr 16, 2015

miloyip commented Apr 16, 2015

RSpliet commented Apr 17, 2015

miloyip commented Apr 17, 2015

miloyip commented Apr 21, 2015

KumarShrawan commented Sep 14, 2015

miloyip commented Oct 1, 2016

Crash when passing nearly-empty document #301

Crash when passing nearly-empty document #301

Comments

RSpliet commented Apr 14, 2015

miloyip commented Apr 14, 2015

pah commented Apr 14, 2015

pah commented Apr 14, 2015

miloyip commented Apr 15, 2015

RSpliet commented Apr 15, 2015

pah commented Apr 15, 2015

RSpliet commented Apr 15, 2015

miloyip commented Apr 16, 2015

RSpliet commented Apr 16, 2015

pah commented Apr 16, 2015

miloyip commented Apr 16, 2015

miloyip commented Apr 16, 2015

RSpliet commented Apr 17, 2015

miloyip commented Apr 17, 2015

miloyip commented Apr 21, 2015

KumarShrawan commented Sep 14, 2015

miloyip commented Oct 1, 2016