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

Cmake cpp14 #13034

Merged
merged 3 commits into from Sep 20, 2019

Conversation

@wsnipex
Copy link
Member

commented Nov 10, 2017

rebased #10864 and added depends check

note: the test commit needs to be dropped before merge

@Paxxi

This comment has been minimized.

Copy link
Member

commented Nov 11, 2017

Just curious, is it Ubuntu or Android holding us back here with gcc versions?

Going up to 17 is obviously an issue but depending on our minimum gcc version std1z could have been an option, gaining some library features

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Nov 11, 2017

on the ubuntu side there is only 14.04 that is a problem, it has only gcc 4.8. Since it's showing it's age on the kernel and mesa side, it's time to drop support anyway(I already did for nightlies).
Then there is Rbpi, which has gcc 4.9(min required for c++14) since 2 days ago.
All other platforms have newer toolchains.

https://gcc.gnu.org/projects/cxx-status.html shows that std1z is currently only well supported in gcc 7.

@Paxxi

This comment has been minimized.

Copy link
Member

commented Nov 11, 2017

yeah, was thinking more about standard library bits like filesystem etc but it doesn't look much better there so it's a moot point. Will have to be patient 😄

wsnipex referenced this pull request Nov 12, 2017

@wsnipex wsnipex force-pushed the wsnipex:cmake_cpp14 branch 3 times, most recently from 654dcec to ee29503 Nov 29, 2017

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2017

jenkins build this please

@fetzerch fetzerch referenced this pull request Nov 30, 2017
3 of 9 tasks complete
@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2017

jenkins build this please

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 11, 2017

good to go?

@Rechi

This comment has been minimized.

Copy link
Member

commented Dec 11, 2017

There are still some cxx11 and c++11 settings in CMakeLists.txt and in the cmake folder which should be either update to c++14 or removed if not needed any more.

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 11, 2017

jenkins build this please

1 similar comment
@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 12, 2017

jenkins build this please

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 19, 2017

know that tests should be reliable:
jenkins build this please

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Dec 19, 2017

jenkins build this please

@wsnipex

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2017

dumdedeedum
jenkins build this please

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from 1794285 to c835d67 Jan 23, 2018

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 8, 2018

Jenkins build this please

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 16, 2018

Jenkins build this please
Good for merge once build successfully?

@Rechi

This comment has been minimized.

Copy link
Member

commented Feb 17, 2018

no, Windows crash in CVariant with this PR

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from c835d67 to cfd93d6 Apr 9, 2018

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from cfd93d6 to e8bf843 Jun 27, 2018

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from e8bf843 to 6450244 Jun 28, 2018

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from fbdbd0b to f0dee7d Aug 9, 2019

@Rechi

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

Seems like https://forum.kodi.tv/showthread.php?tid=344300 is the same crash on Linux, I experienced on Windows. So most likely not a Windows toolchain bug, but a bug in Kodi code.

@fritsch

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

I also was interested but could not access the provided crashlog, do you still have it?

@Rechi

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

no

@a1rwulf

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

The user has reposted the crashlog, are we sure this crash is caused by the cpp14 switch.
I'm pretty sure I've seen this already with kodi 18 as well as current master.
This is Thread1 of the users crashlog posted in the forum:

Thread 1 (Thread 0x7f41435de700 (LWP 4480)):
#0  0x00007f41f2e830e5 in std::_Rb_tree_insert_and_rebalance(bool, std::_Rb_tree_node_base*, std::_Rb_tree_node_base*, std::_Rb_tree_node_base&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x000055e978e7d793 in CVariant::operator=(CVariant const&) ()
#2  0x000055e9793ce566 in JSONRPC::JSONSchemaTypeDefinition::Set(std::shared_ptr<JSONRPC::JSONSchemaTypeDefinition>) ()
#3  0x000055e9793ee8ff in JSONRPC::JSONSchemaTypeDefinition::Check(CVariant const&, CVariant&, CVariant&) ()
#4  0x000055e9793f0a73 in JSONRPC::JsonRpcMethod::checkParameter(CVariant const&, std::shared_ptr<JSONRPC::JSONSchemaTypeDefinition>, unsigned int, CVariant&, unsigned int&, CVariant&) ()
#5  0x000055e9793f0f39 in JSONRPC::JsonRpcMethod::Check(CVariant const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, bool, JSONRPC::JSONRPC_STATUS (*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, CVariant const&, CVariant&), CVariant&) const ()
#6  0x000055e9793f14e8 in JSONRPC::CJSONRPC::HandleMethodCall(CVariant const&, CVariant&, JSONRPC::ITransportLayer*, JSONRPC::IClient*) ()
#7  0x000055e9793f1de5 in JSONRPC::CJSONRPC::MethodCall(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*) ()
#8  0x000055e978aef776 in CHTTPJsonRpcHandler::HandleRequest() ()
#9  0x000055e9793b8f07 in CWebServer::HandleRequest(std::shared_ptr<IHTTPRequestHandler> const&) ()
#10 0x000055e97939215e in CWebServer::HandlePartialRequest(MHD_Connection*, CWebServer::ConnectionHandler*, HTTPRequest const&, char const*, unsigned long*, void**) ()
#11 0x000055e979387b6e in CWebServer::AnswerToConnection(void*, MHD_Connection*, char const*, char const*, char const*, char const*, unsigned long*, void**) ()
#12 0x00007f41f9a205f5 in ?? () from /usr/lib/x86_64-linux-gnu/libmicrohttpd.so.12
#13 0x00007f41f9a221a8 in ?? () from /usr/lib/x86_64-linux-gnu/libmicrohttpd.so.12
#14 0x00007f41f9a24016 in ?? () from /usr/lib/x86_64-linux-gnu/libmicrohttpd.so.12
#15 0x00007f41f9a27558 in ?? () from /usr/lib/x86_64-linux-gnu/libmicrohttpd.so.12
#16 0x00007f41fb1246db in start_thread (arg=0x7f41435de700) at pthread_create.c:463
#17 0x00007f41f3dbc88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
@a1rwulf

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

For comparison, this is a crash I see sometimes right after startup when a json request is done.
Same place but looks like it crashes differently in the stdlib:

Thread 1 (Thread 0x7fe9a5e6a700 (LWP 901)):
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007fe9ea05f524 in __GI_abort () at abort.c:79
#2  0x00007fe9e9f4e8b2 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/libstdc++.so.6
#3  0x00007fe9e9f4c986 in ?? () from /usr/lib/libstdc++.so.6
#4  0x00007fe9e9f4c9c1 in std::terminate() () from /usr/lib/libstdc++.so.6
#5  0x00007fe9e9f4cbf3 in __cxa_throw () from /usr/lib/libstdc++.so.6
#6  0x00007fe9e9f4d0bc in operator new(unsigned long) () from /usr/lib/libstdc++.so.6
#7  0x00007fe9e9f8afa9 in std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) () from /usr/lib/libstdc++.so.6
#8  0x00007fe9e9f8be1b in std::string::_Rep::_M_clone(std::allocator<char> const&, unsigned long) () from /usr/lib/libstdc++.so.6
#9  0x00007fe9e9f8c4fc in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(std::string const&) () from /usr/lib/libstdc++.so.6
#10 0x0000000000f77926 in CVariant::operator=(CVariant const&) ()
#11 0x0000000000bdeda1 in ?? ()
#12 0x0000000000becfad in ?? ()
#13 0x0000000000beda09 in JSONRPC::JSONSchemaTypeDefinition::Set(std::shared_ptr<JSONRPC::JSONSchemaTypeDefinition>) ()
#14 0x0000000000c071fa in JSONRPC::JSONSchemaTypeDefinition::Check(CVariant const&, CVariant&, CVariant&) ()
#15 0x0000000000c078df in JSONRPC::JSONSchemaTypeDefinition::Check(CVariant const&, CVariant&, CVariant&) ()
#16 0x0000000000c09392 in JSONRPC::JsonRpcMethod::checkParameter(CVariant const&, std::shared_ptr<JSONRPC::JSONSchemaTypeDefinition>, unsigned int, CVariant&, unsigned int&, CVariant&) ()
#17 0x0000000000c096ee in JSONRPC::JsonRpcMethod::Check(CVariant const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, bool, JSONRPC::JSONRPC_STATUS (*&)(std::string const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, CVariant const&, CVariant&), CVariant&) const ()
#18 0x0000000000c09882 in JSONRPC::CJSONServiceDescription::CheckCall(char const*, CVariant const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, bool, JSONRPC::JSONRPC_STATUS (*&)(std::string const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*, CVariant const&, CVariant&), CVariant&) ()
#19 0x0000000000c09d91 in JSONRPC::CJSONRPC::HandleMethodCall(CVariant const&, CVariant&, JSONRPC::ITransportLayer*, JSONRPC::IClient*) ()
#20 0x0000000000c0a01f in JSONRPC::CJSONRPC::MethodCall(std::string const&, JSONRPC::ITransportLayer*, JSONRPC::IClient*) ()
#21 0x00000000011f9b1d in CHTTPJsonRpcHandler::HandleRequest() ()
#22 0x0000000000cc1a8c in CWebServer::HandleRequest(std::shared_ptr<IHTTPRequestHandler> const&) ()
#23 0x0000000000cc1f9b in CWebServer::HandlePartialRequest(MHD_Connection*, CWebServer::ConnectionHandler*, HTTPRequest const&, char const*, unsigned long*, void**) ()
#24 0x0000000000cbeee1 in CWebServer::AnswerToConnection(void*, MHD_Connection*, char const*, char const*, char const*, char const*, unsigned long*, void**) ()
#25 0x0000000001233bf1 in ?? ()
#26 0x0000000001235402 in ?? ()
#27 0x0000000001235f4e in ?? ()
#28 0x00000000012384bf in ?? ()
#29 0x00007fe9ec56efa0 in start_thread (arg=<optimized out>) at pthread_create.c:486
#30 0x00007fe9ea131d5f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
@a1rwulf

This comment has been minimized.

Copy link
Member

commented Sep 1, 2019

@Rechi so is above crash still blocking this particular PR?
I think it's an unrelated issue, that seems hard to track down.
As posted above, I've experienced a similar crash with the current code as well, so it looks unrelated to the c++14 changes to me.

@Paxxi

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

I haven't looked at the code but the stack trace seems like the variant is in a bad state, possibly moved from. I know we had crashes because of that before but I thought I fixed all the cases of that 😀

@a1rwulf

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

I'll try if I can reproduce it somehow.
Right now, I can't really trigger it on my dev machine in a reliable way.

@Paxxi

This comment has been minimized.

Copy link
Member

commented Sep 8, 2019

I'm thinking that this should solve the issue. Modifying the schema definitions during a check isn't thread safe and there's no locking to protect it. Paxxi@164a121

@Rechi

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

@Paxxi this seems to fix the crash. Are you going to create a PR?

@Paxxi

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

Yeah I can do that. Since it was mentioned here I thought it could be picked into this pr

@Rechi

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

As the fix is unrelated to this PR, it should be a separate PR.

@DaveTBlake DaveTBlake referenced this pull request Sep 10, 2019
0 of 13 tasks complete

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from f0dee7d to 2206f30 Sep 14, 2019

@Rechi
Rechi approved these changes Sep 14, 2019
fetzerch and others added 3 commits Sep 16, 2019
[cmake] Enable C++14 support for core
Also remove the FindCXX11.cmake module and make use of CMake
techniques to specify the necessary compiler flags.

@Rechi Rechi force-pushed the wsnipex:cmake_cpp14 branch from 2206f30 to 59433d1 Sep 17, 2019

@Rechi Rechi requested review from notspiff, AlwinEsch, ksooo and peak3d Sep 18, 2019

docs/README.openSUSE.md Show resolved Hide resolved
@@ -1,12 +1,12 @@
cmake_minimum_required(VERSION 3.1)

This comment has been minimized.

Copy link
@AlwinEsch

AlwinEsch Sep 18, 2019

Member

Better change direct to 3.4 (or 3.5) to have equal to the base.

This comment has been minimized.

Copy link
@Rechi

Rechi Sep 18, 2019

Member

This PR is about switching to C++14, not about unifying cmake_minimum_required.
Version 3.1 is enough for this change.

@wsnipex wsnipex merged commit 35e6ca1 into xbmc:master Sep 20, 2019

1 check passed

default You're awesome. Have a cookie
Details

@Rechi Rechi added this to the Matrix 19.0-alpha 1 milestone Sep 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.