rados+rpm: Update symver defs and re-enable LTO

[b-ranto]

The gcc compiler now supports symver attribute. We should update the symvers to be able to support LTO on the platforms that support the symver attribute.

Fixes: https://tracker.ceph.com/issues/40060

Checklist

• References tracker ticket
• Updates documentation if necessary
• Includes tests for new functionality or reproducer for bug

---

Show available Jenkins commands

• jenkins retest this please
• jenkins test classic perf
• jenkins test crimson perf
• jenkins test signed
• jenkins test make check
• jenkins test make check arm64
• jenkins test submodules
• jenkins test dashboard
• jenkins test api
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox

[b-ranto]

Fixes: https://tracker.ceph.com/issues/40060

[b-ranto]

It looks like we can't do this just yet. There is no support for the attribute in the older versions of gcc (it might have been introduced in GCC 10 iirc).

Maybe, we could do this conditionally -- check if the attribute is supported and fall-back to asm definitions if it is not?

[b-ranto]

OK, I have implemented conditional use of the attribute. It will only be used if the compiler supports the symver attribute.

[badone]

LGTM

[smithfarm]

@b-ranto Thanks for this! Could you add

Fixes: https://tracker.ceph.com/issues/40060

to one of the commits so the git history contains a link to the Redmine issue? (Maybe librados/librados_c: Use symver attribute if available is the most relevant?)

[smithfarm]

Also, if it's not too much trouble, could we hold off from merging this until I test that it doesn't break the openSUSE Factory build?

[b-ranto]

@smithfarm Sure, we can wait for the openSUSE fatctory build -- that is why I was asking for the suse_version from which we can enable LTO. :-)

Anyway, I did put the fixes line to the commits before, I just forgot to add it to the latest commits, I have updated those too, now.

[smithfarm]

@b-ranto Thanks! Would it make sense (and be worth the trouble) to mention Fixes: https://tracker.ceph.com/issues/40060 in the PR description as well?

[smithfarm]

@b-ranto master, quincy, and pacific only target openSUSE Leap 15.3/SLE-15-SP3 and above. So if this patchset works fine in SLE-15-SP3 and Factory, there won't be any need to add a suse_version guard to ceph.spec.in for this PR.

I will trigger a test build on SLE-15-SP3 and Factory now.

[b-ranto]

@smithfarm Sounds good, AFAIK, the support for the attribute landed in GCC 10 so if you use GCC 10 in these, you should be good.

[smithfarm]

@smithfarm Sounds good, AFAIK, the support for the attribute landed in GCC 10 so if you use GCC 10 in these, you should be good.

Yeah, I got that part. What I'm looking at now is whether the cmake code and guards that are being added here are enough for this patch to work in SLE-15-SP3, which has gcc 7.

[smithfarm]

The reason I'm looking at SLE-15-SP3 at all is because the Redmine ticket says this is going to be backported to pacific.

If that's wrong, and this is not going to be backported to pacific, then my task will be much simpler, since I'll only have to build it for openSUSE Factory :-)

[b-ranto]

Yes, it is also targeted for pacific. You will definitelly need to keep the bits that disable LTO for SLE-15-SP3 then.

[b-ranto]

jenkins test make check

[tchaikov]

https://shaman.ceph.com/builds/ceph/wip-kefu-testing-2021-08-06-1232/9c50b6e8c95db4d21bbe63cadf31002177e50994/

[tchaikov]

just for the record, __attribute__((__symver__ ...)) is not supported by any of the compilers we are using when building packages in our CI at this moment. none of the versions of used gcc is greater or equal to 10 so far.

-- Performing Test HAVE_ATTR_SYMVER
-- Performing Test HAVE_ATTR_SYMVER - Failed
-- Performing Test HAVE_ASM_SYMVER
-- Performing Test HAVE_ASM_SYMVER - Success

[b-ranto]

Yeah, this was a RHEL 9 ~requirement:

https://bugzilla.redhat.com/show_bug.cgi?id=1986190

[wjwithagen]

@b-ranto @tchaikov

This fails on FreeBSD/Clang.
Got to figure out where that refence is declared and why it is not available.
Problem is that not all gnu-ld tricks are available in llvm-ld.

/usr/local/bin/ld: CMakeFiles/librados-config.dir/librados-config.cc.o: in function `main':
/home/jenkins/workspace/ceph-master-compile/src/librados-config.cc:46: undefined reference to `rados_version'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_nobjects_list_next2'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_conf_parse_argv_remainder'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_aio_create_completion'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_create_with_context'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_create2'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_aio_get_return_value'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_conf_parse_env'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_aio_create_completion2'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_conf_set'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_create'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_ioctx_create2'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_conf_parse_argv'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_conf_read_file'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_ioctx_create'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_nobjects_list_seek_cursor'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_nobjects_list_seek'
/usr/local/bin/ld: ../lib/librados.so.2.0.0: undefined reference to `rados_nobjects_list_open'
c++: error: linker command failed with exit code 1 (use -v to see invocation)

[b-ranto]

@wjwithagen Can you share the output of the ./do_cmake.sh step (or equivalent for FreeBSD)? I am mostly interested in the HAVE_ATTR_SYMVER check. Does (your version of) llvm support attribute symver? If it does not, will it at least produce a warning for an unknown attribute?

[wjwithagen]

@b-ranto
I think it does not:

/home/jenkins/workspace/ceph-master-compile/src/librados/librados_c.cc:57:48: note: expanded from macro 'LIBRADOS_C_API_BASE_DEFAULT'
  extern __typeof (_##fn) _##fn __attribute__((__symver__ (#fn "@@")))
                                               ^~~~~~~~~~~~~~~~~~~~~
/home/jenkins/workspace/ceph-master-compile/src/librados/librados_c.cc:263:1: warning: unknown attribute '__symver__' ignored [-Wunknown-attributes]
LIBRADOS_C_API_BASE_DEFAULT(rados_get_min_compatible_client);

But the ./do_cmake thinks different:

-- Performing Test HAVE_ATTR_SYMVER
-- Performing Test HAVE_ATTR_SYMVER - Success
-- Performing Test HAVE_ASM_SYMVER
-- Performing Test HAVE_ASM_SYMVER - Success

So I conditionalised HAVE_ASM_SYMVER:

#if defined(HAVE_ATTR_SYMVER) && !defined(__FreeBSD__)

Which works....

[b-ranto]

Yeah, gcc also only warns on that, we pass -Werror=attribute to the compiler to make it an error. It looks like llvm has a different option for that. We could probably change the flag to simply -Werror. Does llvm support that? Can you check please? You can change that, here:

https://github.com/ceph/ceph/blob/master/cmake/modules/CephChecks.cmake#L151

[tchaikov]

for clang, it should be -Wunknown-attributes.

[tchaikov]

lemme create a patch.

[tchaikov]

#42698

@b-ranto @wjwithagen

[wjwithagen]

-Werror does work
-Wunknown-attributes does not help. I guess that it does generate a warning, but the warning is not yet promoted to error.
So it would require -Werror -Wunknown-attributes
Which leads to using -Werror I suggest.

[tchaikov]

-Werror does work
-Wunknown-attributes does not help. I guess that it does generate a warning, but the warning is not yet promoted to error.
So it would require -Werror -Wunknown-attributes
Which leads to using -Werror I suggest.

@wjwithagen just wanted to explain the option name, not to propose a solution in my previous comment. if we really go with a single option like Boris put in his change, then it should be -Werror-unknown-attributes. but i'd trade a solution with two special cases for a simpler but probably obscure one .