Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix g++6 compile error #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cryptokat
Copy link

this error would be emitted when compiling with g++6

/pebblesdb/src/db/db_test.cc: In function ‘void leveldb::print_timer_info(std::__cxx11::string, uint64_t, uint64_t)’:                                                             
/pebblesdb/src/db/db_test.cc:2237:25: error: call of overloaded ‘abs(uint64_t)’ is ambiguous
  uint64_t diff = abs(a-b);  

@vijay03
Copy link
Member

vijay03 commented Jan 18, 2019

Hi @cryptokat thanks for the PR! If abs() doesn't link properly in g++ 6, I think the right move is to find the g++6 specific version of the function, and include it with ifdefs. Removing abs causes tests to fail: we do want the absolute value there.

@cryptokat
Copy link
Author

@vijay03 How does it make sense to use abs here when both a and b are unsigned?

@vijay03
Copy link
Member

vijay03 commented Jan 18, 2019

I might be missing something here, but even with unsigned, if b is greater than a, a - b results in a large number instead of the difference which we want.

A hacky way to do this would be:

uint64_t a, b, diff;
a = 1; b = 3;
if (a > b) { diff = a - b; } else diff = b - a;

@cryptokat cryptokat force-pushed the cryptokat-patch-1 branch 2 times, most recently from 8d47945 to cebee6e Compare January 18, 2019 02:20
this error would be emitted when compiling with g++6
```
/pebblesdb/src/db/db_test.cc: In function ‘void
leveldb::print_timer_info(std::__cxx11::string, uint64_t, uint64_t)’:
/pebblesdb/src/db/db_test.cc:2237:25: error: call of overloaded
‘abs(uint64_t)’ is ambiguous
  uint64_t diff = abs(a-b);
```
@cryptokat
Copy link
Author

It turns out that DBTest.MultiThreaded crashes occasionally because VersionSet::RemoveFileLevelBloomFilterInfo isn't thread-safe.

Here's a stack trace:

==== Test DBTest.MultiThreaded
[New Thread 0x7fff4cfd9700 (LWP 23220)]
[New Thread 0x7fff4c7d8700 (LWP 23221)]
[New Thread 0x7fff4bfd7700 (LWP 23222)]
... starting thread 0
[New Thread 0x7fff4b7d6700 (LWP 23223)]
... starting thread 1
[New Thread 0x7fff4afd5700 (LWP 23224)]
... starting thread 2
[New Thread 0x7fff4a7d4700 (LWP 23225)]
... starting thread 3

Thread 307 "db_test" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff4c7d8700 (LWP 23221)]
0x00007ffff7afe106 in std::_Rb_tree_rebalance_for_erase(std::_Rb_tree_node_base*, std::_Rb_tree_node_base&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) where
#0  0x00007ffff7afe106 in std::_Rb_tree_rebalance_for_erase(std::_Rb_tree_node_base*, std::_Rb_tree_node_base&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00005555555d46e3 in std::_Rb_tree<unsigned long, std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*>, std::_Select1st<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::_M_erase_aux(std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >) ()
#2  0x00005555555d2919 in std::_Rb_tree<unsigned long, std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*>, std::_Select1st<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::erase[abi:cxx11](std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >) ()
#3  0x00005555555d0a64 in std::_Rb_tree<unsigned long, std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*>, std::_Select1st<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::_M_erase_aux(std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >) ()
#4  0x00005555555cd5e9 in std::_Rb_tree<unsigned long, std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*>, std::_Select1st<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::erase[abi:cxx11](std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::_Rb_tree_const_iterator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >) ()
#5  0x00005555555c8e3c in std::_Rb_tree<unsigned long, std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*>, std::_Select1st<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::erase(unsigned long const&) ()
#6  0x00005555555c6273 in std::map<unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*> > >::erase(unsigned long const&) ()
#7  0x00005555555b9358 in leveldb::VersionSet::RemoveFileLevelBloomFilterInfo(unsigned long) ()
#8  0x0000555555591e7f in leveldb::DBImpl::DeleteObsoleteFiles() ()
#9  0x0000555555595600 in leveldb::DBImpl::BackgroundCompactionGuards(leveldb::FileLevelFilterBuilder*) ()
#10 0x0000555555594dc3 in leveldb::DBImpl::CompactLevelThread() ()
#11 0x000055555559e395 in leveldb::DBImpl::CompactLevelWrapper(void*) ()
#12 0x00005555555e7403 in leveldb::(anonymous namespace)::StartThreadWrapper(void*) ()
#13 0x00007ffff7326494 in start_thread (arg=0x7fff4c7d8700) at pthread_create.c:333
#14 0x00007ffff7068acf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:97

@vijay03
Copy link
Member

vijay03 commented Jan 18, 2019

Thanks for catching this! Could you report this as an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants