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

Upgrade to 15.0.0 #31

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from

Conversation

mamtashukla
Copy link

This series of patch allows to build compiler-rt for 15.0.0 version:
1d75a0a patches: remove patch for __is_convertible
f172b04 patches: add patch to remove abi-tag
a5178e9 Makefile.uk: Use -fpermissive flag
8ae7efe Makefile.uk: Upgrade to 15.0.0

Fixes: unikraft/lib-compiler-rt#7

@razvand razvand self-assigned this Aug 5, 2023
@razvand razvand added the enhancement New feature or request label Aug 5, 2023
Copy link
Contributor

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some observations @mamtashukla:

  1. The changes compile with gcc-11 only if the CXX_THREADS option is enabled. The compile errors look something like this:
In file included from /home/maria/arm/apps/app-helloworld-cpp/build/libcxxabi/origin/libcxxabi-15.0.0.src/src/cxa_guard.cpp:15:
/home/maria/arm/apps/app-helloworld-cpp/build/libcxxabi/origin/libcxxabi-15.0.0.src/src/cxa_guard_impl.h:288:8: error: ‘__libcpp_mutex_t’ in namespace ‘std’ does not name a type
288 |   std::__libcpp_mutex_t mutex = _LIBCPP_MUTEX_INITIALIZER;
 |        ^~~~~~~~~~~~~~~~

There are a multitude of errors regarding mutexes, conditional variables etc. My guess is that this is due to the placement of the libs in the Makefile:

LIBS := $(UK_LIBS)/lib-libcxxabi:$(UK_LIBS)/lib-libcxx:$(UK_LIBS)/lib-compiler-rt:$(UK_LIBS)/lib-libunwind:$(UK_LIBS)/lib-musl

Basically, AFAIK, C++ mutexes and varconds need pthread support, and musl being compiled the last doesn't help with that. I don't know how to fix this, as moving musl in front of libcxx brings other compiler errors 🥲

  1. For the patch commits please use capital letter after the selector, eg: "patches: Add patch to remove abi-tag"

  2. Rename your newly added patch with 0001-*, since you are deleting the old one with this tag.

LE: The first point is now redundant, as the error is part of libcxxabi, and I've already suggested some changes there to fix it. @mamtashukla you still need to address the last 2 points 😄

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mamtashukla.
Please rename the patches (so we have 0001 first, even if you remove the current one).
Also please use git format-patch to create the patches. You should clone the origin libcxx repository, do the changes from the patch, make a commit and then run format-patch, you would end up with something similar to the patches here

@mamtashukla
Copy link
Author

mamtashukla commented Aug 6, 2023

v2:
-Use capital letter for commit after selector
08e75d2 patches: Remove patch for __is_convertible
ccd8b56 patches: Add patch to remove abi-tag

-Fix patch format

Copy link
Contributor

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good on my side, now, thanks for the prompt update @mamtashukla!

Reviewed-by: Maria Sfiraiala [email protected]

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some conflicts @mamtashukla, can you please rebase this on top of staging?

ABI_TAG was introduced in upstream:
llvm/llvm-project@67b0b02

These ABI_TAG leads to error:

build/libcxx/origin/libcxx-15.0.0.src/include/__support/musl/xlocale.h:38:74:
error: ‘abi_tag’ attribute applied to extern "C" declaration ‘long long
int wcstoll_’
   38 | wcstoll_l(const wchar_t *__nptr, wchar_t **__endptr, int __base,
locale_t) {

Signed-off-by: Mamta Shukla <[email protected]>
__is_convertible patch does not applies to 15.0.0 version.

It is possible to compile libcxx without this patch. Tested for
gcc-11.

This reverts the commit:
4dd2450: patches: Backport upstream fix for __is_convertible

Signed-off-by: Mamta Shukla <[email protected]>
Use -fpermissive flag to allow compiler-rt build

note: (if you use ‘-fpermissive’, G++ will accept your code, but
allowing the use of an undeclared name is deprecated)

build/libcxx/origin/libcxx-15.0.0.src/include/__type_traits/is_same.h:22:72:
error: template argument 1 is invalid
   22 | struct _LIBCPP_TEMPLATE_VIS is_same :
_BoolConstant<__is_same(_Tp, _Up)> { };

Signed-off-by: Mamta Shukla <[email protected]>
@mamtashukla
Copy link
Author

There are some conflicts @mamtashukla, can you please rebase this on top of staging?

Rebased on top of staging and tested with a rebuild.

@mamtashukla mamtashukla requested a review from StefanJum August 17, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Update library to version 15.0.0
4 participants