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

gcc10 cxx11 abi issue with CLING #32497

Closed
dan131riley opened this issue Dec 15, 2020 · 15 comments
Closed

gcc10 cxx11 abi issue with CLING #32497

dan131riley opened this issue Dec 15, 2020 · 15 comments

Comments

@dan131riley
Copy link

DataFormats/FWLite/test/triggernames_cint.C is failing in gcc10 IBs with the error

IncrementalExecutor::executeFunction: symbol '_ZNK3edm12TriggerNames4sizeEv' unresolved while linking function '_GLOBAL__sub_I_cling_module_10'!
You are probably missing the definition of edm::TriggerNames::size() const
Maybe you need to load the corresponding shared library?
size = 47268205522688

The problem appears to be a change in cxx11 ABI propagation in gcc10. edm::TriggerNames::size() is defined as:

typedef std::vector<std::string> Strings;
Strings::size_type size() const;

If I run 'nm -C' on the resulting library and look for the routine, I get

000000000000aa50 T edm::TriggerNames::size[abi:cxx11]() const

and CLING doesn't find it. If I change the definition to:

std::vector<int>::size_type size() const;

I get

000000000000aa50 T edm::TriggerNames::size() const

and the test runs fine. If I do

std::vector<std::string>::size_type size() const;
std::vector<int>::size_type TriggerNames::size() const { return triggerNames_.size(); }

it compiles ok but fails at link time with a linker undefined reference

TriggerResultsByName.cc:(.text+0x8ee): undefined reference to `edm::TriggerNames::size[abi:cxx11]() const'

So in gcc10, std::vector<std::string>::size_type and std::vector<int>::size_type are the same type to the compiler but have different ABI linkage, and CLING doesn't know about this.

This appears to be new behavior in gcc10. but I haven't found it documented. It also seems weird to me, and may bite us in other ways.

@cmsbuild
Copy link
Contributor

A new Issue was created by @dan131riley Dan Riley.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@Dr15Jones
Copy link
Contributor

Weird, I thought return values are not part of the ABI since they can be ignored.

@makortel
Copy link
Contributor

Adding @vgvassilev in case the "old" version of LLVM (5) used for rootcling could play any role (we encountered a case where a piece of code failed to compile with rootcling in #30906).

@vgvassilev
Copy link
Contributor

After a chat with @zygoloid we believe it is a bug in gcc. If T and U are the same type, then T size() const and U size() const are the same function, and must have the same mangling.

We should report this bug to gcc, if it was not reported.

@vgvassilev
Copy link
Contributor

That's the godbolt link. The bug is present from gcc10 onward. Clang seems to agree with gcc9.

@makortel
Copy link
Contributor

@dan131riley Could you follow up with gcc (if you didn't yet)?

In the meantime, the issue is worked round in #32528.

@dan131riley
Copy link
Author

@makortel yes I'll followup with a gcc bug report, probably tomorrow

@dan131riley
Copy link
Author

submitted https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98481

@makortel
Copy link
Contributor

makortel commented Jan 6, 2021

@dan131riley Given the (already merged) workaround #32528, is it still beneficial to keep this issue open or could we close it?

@dan131riley
Copy link
Author

We have a workaround and an open bug report with the gcc developers, so there's no need to keep this ticket open.

@makortel
Copy link
Contributor

makortel commented Jan 6, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2021

This issue is fully signed and ready to be closed.

@dan131riley
Copy link
Author

gcc devs agreed it’s a bug, fix and regression test are now on the gcc trunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants