Added zmalloc from redis in order to support different allocators

[thedrow]

There's still a failing test that I can't track. Care to help?

[c9s]

which test case?

[c9s]

Yeah, two test cases are failed:

check_tree.c:274:F:testcase:test_pcre_pattern_simple:0: Assertion 'matched' failed check_tree.c:307:F:testcase:test_pcre_pattern_more:0: Assertion 'matched' failed

can you take a look?

also please merge our master HEAD. thanks :)

[c9s]

You can fetch the branch thedrow-topic/zmalloc, in which I have merged your changes with the master HEAD.

[thedrow]

I just rebased.

[thedrow]

I'll take yours then. Why is it failing? I have no idea.

[c9s]

build result: https://travis-ci.org/c9s/r3/jobs/25624754

[c9s]

you need to update configure.ac 0211a7f and check flags used in src/Makefile.am , tests/Makefile.am

[c9s]

after you updated the configure.ac and Makefile.am, you should run autoconf, automake

[thedrow]

Wait, the whole point of zmalloc is to make jemalloc/tcmalloc optional. We need to detect if any of them are installed already and configure the config.h accordingly.

[thedrow]

I really don't understand the testrunner output. I'm used to python where I get the line the error occurred. Can you point me to the right direction?

[c9s]

You can run tests/check_tree separately, or use gdb to debug it, the binary should be in tests/.libs or something.

[c9s]

If its a segfault, gdb 'bt' backtrace shall help u find the correct entry point

[thedrow]

Please assume I don't know anything and explain in details because I've never once debugged through a command line in my life :P

[thedrow]

Hmm ubuntu doesn't install the pc (used by pkg-config) file for jemalloc. Is there a way to fix it?

[thedrow]

Ok, so the build process should generate a pc file much like https://github.com/openwebos/jemalloc/blob/master/files/pkgconfig/jemalloc.pc.in. i just made up something that works.

[thedrow]

I run the program using gdb and bt says no stack afterwards.

[thedrow]

So it's not a segfault and valgrind says no leak is found. What else?

[thedrow]

Here is the valgrind log: https://gist.github.com/thedrow/3706961dbe5e516832ff

[c9s]

It seems only pcre related tests failed?

[c9s]

And edge.c does not know zmalloc function (see the gcc warnings)

[thedrow]

Could it be because it uses another malloc and not jemalloc?

[thedrow]

And I don't get any warning about edge.c.

[c9s]

Hi! I am back, try running ./configure with --enable-debug which adds the -ggdb flags to the build process.

and you need to make sure any source file calling z* functions should include the "zmalloc.h"

[c9s]

And you're calling zcalloc without element size * element numbers, I saw there is only element numbers.

[c9s]

You need to call zcalloc just like this:

zcalloc(sizeof(int) * 10) not just zcalloc(10)

https://github.com/thedrow/r3/blob/topic/zmalloc/src/node.c#L161

[thedrow]

But that one is a char no?

[thedrow]

I just did the changes locally and it has no effect.

[c9s]

Yes, it's char.

[c9s]

Well, I think you need to correct the allocation calls and commit them. and see what else we can do.

[c9s]

There are two calls actually:

src/node.c:160:    cpat = zcalloc(128);
src/node.c:192:    n->ov = (int*) zcalloc(n->ov_cnt);

[thedrow]

I saw those. I get the same errors. I'll commit in an hour.

[thedrow]

Btw, is there a reason why cpat is not freed at the end of the method?

[c9s]

the cpat means combined_pattern which is used for pcre and tree dumping & debugging (we can see the compiled regexp)

[c9s]

https://gist.github.com/c9s/e0ef1f0a1f78e6adedb2 I saw a lot of compiler warnings, which might cause the matching fails. you might need to fix them all

[c9s]

You may fetch my commit de5a308 on the thedrow-topic/zmalloc branch which fixes most warnings.

[thedrow]

Fetched and done.

[c9s]

OK, I've checked the last build failing message: https://travis-ci.org/c9s/r3/jobs/25671096

seems like it calls zfree at wrong pointer:

/lib/x86_64-linux-gnu/libc.so.6(+0x7e626)[0x2ae981b4e626]
/home/travis/build/c9s/r3/src/.libs/libr3.so.0(zfree+0x52)[0x2ae9818b4aa2]
/home/travis/build/c9s/r3/src/.libs/libr3.so.0(r3_edge_branch+0x129)[0x2ae9818b3d29]
/home/travis/build/c9s/r3/src/.libs/libr3.so.0(r3_tree_insert_pathl_+0x18e)[0x2ae9818b38ee]
/home/travis/build/c9s/r3/tests/.libs/lt-check_tree[0x403f20]

And here are the failing test cases (segment faults)

check_tree.c:185:E:testcase:testr3_tree_insert_pathl:0: (after this point) Received signal 6 (Aborted)
check_tree.c:45:E:testcase:test_compile:0: (after this point) Received signal 6 (Aborted)
check_tree.c:265:E:testcase:test_pcre_pattern_simple:0: (after this point) Received signal 6 (Aborted)
check_tree.c:284:E:testcase:test_pcre_pattern_more:0: (after this point) Received signal 11 (Segmentation fault)
check_tree.c:115:E:testcase:test_pcre_patterns_insert:0: (after this point) Received signal 6 (Aborted)
check_tree.c:125:E:testcase:test_pcre_patterns_insert_2:0: (after this point) Received signal 6 (Aborted)
check_tree.c:144:E:testcase:test_pcre_patterns_insert_3:0: (after this point) Received signal 6 (Aborted)
check_tree.c:363:E:testcase:benchmark_str:0: (after this point) Received signal 6 (Aborted)

You may comment all but keep one test case to narrow down the problem.

I guess the key is inside the edge_branch function.

[thedrow]

Oh crap. This is from the workplace's user... wait.

[thedrow]

Well for some reason I can't replace it. Nevermind...

[c9s]

it's OK :)

[c9s]

seems like we have other places using strndup instead of zstrndup?

src/edge.c:58:    e1 = r3_edge_create(strndup(s1, s1_len), s1_len, new_child);
src/edge.c:75:    e->pattern = strndup(e->pattern, dl);
src/node.c:309:                    str_array_append(entry->vars , strndup(substring_start, substring_length));
src/node.c:462:            r3_node_add_child(n, strndup(path, (int)(p - path)), child);
src/node.c:469:            r3_node_add_child(n, strndup(path, path_len) , child);

[c9s]

I saw there is a zstrdup in zmalloc.c but there is no zstrndup, we need to implement one.

[thedrow]

What's the difference?

[thedrow]

@c9s Got it. What's next?

[c9s]

zstrndup is to avoid lenght calculation

[thedrow]

So why is this still failing?

[c9s]

You may check the incompatible pointer warnings.

Also there is another segment fault in tree_free function.

[c9s]

Did we set NULL to pcre related variables when creating node or tree?

[thedrow]

I have no idea. Can you please try to debug this since I'm a bit lost.

[thedrow]

As far as I can see all of them are set to NULL when creating both node and tree.