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 build against musl libc #899

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

Conversation

ismaell
Copy link

@ismaell ismaell commented Sep 9, 2022

Description

Fix build against musl libc.

Probably MALLOC_UNIXLIKE_OVERLOAD_ENABLED only works with glibc, so use __GLIBC__ in addition to __linux__ to define it.

  • - git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details)

Type of change

Choose one or multiple, leave empty if none of the other choices apply

Add a respective label(s) to PR if you have permissions

  • bug fix - change that fixes an issue
  • new feature - change that adds functionality
  • tests - change in tests
  • infrastructure - change in infrastructure and CI
  • documentation - documentation update

Tests

  • added - required for new features and some bug fixes
  • not needed

Documentation

  • updated in # - add PR number
  • needs to be updated
  • not needed

Breaks backward compatibility

  • Yes
  • No
  • Unknown

Notify the following users

Other information

Probably MALLOC_UNIXLIKE_OVERLOAD_ENABLED only works with glibc, so use
__GLIBC__ in addition to __linux__ to define it.
@stefson
Copy link

stefson commented Nov 14, 2022

can you link back to a build log of the failed compile, or a bug report where this is discussed? I was unable to reproduce with tbb-2021.7.0

@ismaell
Copy link
Author

ismaell commented Nov 14, 2022

The error:

In file included from .../oneTBB/test/common/test.h:32,
                 from .../oneTBB/test/tbbmalloc/test_malloc_overload.cpp:45:
.../oneTBB/test/tbbmalloc/test_malloc_overload.cpp: In function 'void DOCTEST_ANON_FUNC_41()':
.../oneTBB/test/tbbmalloc/test_malloc_overload.cpp:400:13: error: 'mallopt' was not declared in this scope; did you mean 'malloc'?
  400 |     REQUIRE(mallopt(0, 0)); // add dummy mallopt call for coverage
      |             ^~~~~~~
.../oneTBB/test/common/doctest.h:1997:9: note: in definition of macro 'DOCTEST_WRAP_IN_TRY'
 1997 |         x;                                                                                         \
      |         ^
.../oneTBB/test/common/doctest.h:2249:9: note: in expansion of macro 'DOCTEST_ASSERT_IMPLEMENT_2'
 2249 |         DOCTEST_ASSERT_IMPLEMENT_2(assert_type, __VA_ARGS__);                                      \
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
.../oneTBB/test/common/doctest.h:2268:30: note: in expansion of macro 'DOCTEST_ASSERT_IMPLEMENT_1'
 2268 | #define DOCTEST_REQUIRE(...) DOCTEST_ASSERT_IMPLEMENT_1(DT_REQUIRE, __VA_ARGS__)
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~
.../oneTBB/test/common/doctest.h:2743:22: note: in expansion of macro 'DOCTEST_REQUIRE'
 2743 | #define REQUIRE(...) DOCTEST_REQUIRE(__VA_ARGS__)
      |                      ^~~~~~~~~~~~~~~
.../oneTBB/test/tbbmalloc/test_malloc_overload.cpp:400:5: note: in expansion of macro 'REQUIRE'
  400 |     REQUIRE(mallopt(0, 0)); // add dummy mallopt call for coverage
      |     ^~~~~~~
compilation terminated due to -Wfatal-errors.
make[2]: *** [test/CMakeFiles/test_malloc_overload.dir/build.make:76: test/CMakeFiles/test_malloc_overload.dir/tbbmalloc/test_malloc_overload.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:4110: test/CMakeFiles/test_malloc_overload.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

@stefson
Copy link

stefson commented Nov 14, 2022

so this is a test failure?

@ismaell
Copy link
Author

ismaell commented Nov 14, 2022

Musl doesn't provide mallopt nor mallinfo, so I guess it's more than that.

@ismaell
Copy link
Author

ismaell commented Nov 14, 2022

In any case, assuming anything based on __linux__ alone is inappropriate.

@ismaell
Copy link
Author

ismaell commented Nov 14, 2022

Musl does provide some non-standard bits like function malloc_usable_size, but it doesn't (and will never) provide any test macros.

@pavelkumbrasev
Copy link
Contributor

In any case, assuming anything based on __linux__ alone is inappropriate.

Is there specific macro for musl? If no, is it possible to pass it through cmake?

@ismaell
Copy link
Author

ismaell commented Nov 16, 2022

In any case, assuming anything based on __linux__ alone is inappropriate.

Is there specific macro for musl? If no, is it possible to pass it through cmake?

No, musl doesn't define any such macros, intentionally.

Isn't this GLIBC-specific anyway?

The proper way would be to test the implementation to see if it's there, then define a macro based on that... but if that's not desirable, relying on the __GLIBC__ macro is a better way than relying on __linux__, in fact, these interfaces should be available with Glibc on non-Linux systems too...

@pavelkumbrasev
Copy link
Contributor

It seems that mallopt and mallinfo implemented in __GLIBC__ so we can use proposed approach.
@ismaell Could you please update a copyright year from 2022 to 2023?

Copy link
Contributor

@ldorau ldorau left a comment

Choose a reason for hiding this comment

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

@ismaell Could you please update a copyright year from 2022 to 2023?

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

4 participants