Mark fcontext asm functions as hidden visibility

[rdoeffinger]

These functions should not be exported as dynamic
symbols if a library uses boost.

Note: I have no idea which if any changes armasm, armclang or xcoff assembler targets would need. It is also very lightly tested, essentially only the arm64_aapcs_macho target at this point.
So maybe more of a bug report/discussion point at this stage.

[dixyes]

There are many softwares using boost context asm codes to implement their own context switching, this may be a huge break change for them.

[rdoeffinger]

Are you maybe confusing it with the properly namespaced C++ functions like boost::context::make_fcontext? (I admit I can't find its definition right now, but that seems to be the one documented, not the C variant).
Either way the name make_fcontext is far too generic to be acceptable to export, it would at least have to be something like boost_context_make_fcontext.

[dixyes]

Are you maybe confusing it with the properly namespaced C++ functions like boost::context::make_fcontext? (I admit I can't find its definition right now, but that seems to be the one documented, not the C variant). Either way the name make_fcontext is far too generic to be acceptable to export, it would at least have to be something like boost_context_make_fcontext.

I agree this, but

https://github.com/php/php-src/blob/39ae00fa0a72e7668b419399567c9709160e0bbb/Zend/zend_fibers.c#L176

[rdoeffinger]

Any proposal how to approach it?
The code you linked to has made a copy of the asm files, so they will not be affected (even if they update the code, they will actually profit from this change). Anyone linking statically will also not be affected.
In fact, in both of these cases users will BENEFIT from this even further, as their internal copies are no longer at risk of being overridden by symbols provided by a dynamically linked libboost, which might be incompatible.
Also as this example shows, anyone using these today would have to manually write a declaration for these functions, and they would not notice if boost had changed the code at compile-time, risking corruption at runtime.
In other words, I think this can only affect really irresponsible developers.
IMO for quality reasons, protecting users that do things correctly must take priority over compatibility with incorrect and obviously irresponsible use (if such even exists).
I would however agree that if there is a use-case for those functions, it would be good to wrap them in C++ functions providing this functionality while also being part of the official API - though not sure if making this an official API is desirable.
But, as so far anyone negatively impacted by this is purely hypothetical (as said, the example you provided would not have any issue but would in fact benefit), I am not sure it's really worth worrying about?

[olk]

Unfortunately, tests fail for on MIPS and RISCV64:

  /usr/bin/ld: test_fiber.cpp:(.text+0x69c4): undefined reference to `jump_fcontext'
  /usr/bin/ld: test_fiber.cpp:(.text+0x6a08): undefined reference to `ontop_fcontext'
  /usr/bin/ld: test_fiber.cpp:(.text+0x6e20): undefined reference to `make_fcontext'

[olk]

Unfortunately, tests fail for on MIPS and RISCV64:

  /usr/bin/ld: test_fiber.cpp:(.text+0x69c4): undefined reference to `jump_fcontext'
  /usr/bin/ld: test_fiber.cpp:(.text+0x6a08): undefined reference to `ontop_fcontext'
  /usr/bin/ld: test_fiber.cpp:(.text+0x6e20): undefined reference to `make_fcontext'

probably caused by misconfiguration of CI-tests

[olk]

ty

[pdimov]

This change seems to break Coroutine and Fiber on macOS.

https://github.com/boostorg/boost/actions/runs/11232703851

[rdoeffinger]

I am confused how it works anywhere actually?
It does not work because fiber_fcontext.hpp is a header which calls make_fcontext etc.
And while these are referred to as detail::make_fcontext, that is misleading. Due to extern "C" the symbol is the plain make_fcontext.
One option is to revert this and instead change the name to something more descriptive like boost_context_make_fcontext.
But in a C++ project like boost it seems a bit non-nice to export plain C symbols, without the type safety of name mangling etc.
So a more involved option would be:

• change the include/boost/context/detail/fcontext.hpp declaration to declare a real C++ function
• define that function in e.g. src/fiber.cpp, also add the extern "C" make_fcontext declarations there and make the C++ make_fcontext call the C version

I guess short-term reverting might be the way to go to not leave things broken in develop.

[rdoeffinger]

This is clearly a hack misusing an existing cpp file, but this approach at least compiles on macOS:
068bed6
I can try to find time to polish it more if that seems a sensible way to go.
(I confess I have not run the tests on it yet, because it's late and I forgot again how to run tests and could not find it quickly)

[rdoeffinger]

Ran ./b2 libs/context/test and got only passes.
Same for libs/coroutine/test and libs/fiber/test

[olk]

I can try to find time to polish it more if that seems a sensible way to go.

That would be nice ...