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

__clang_cuda_runtime_wrapper.h __THROW macro redefinition inconsistent with libc #47213

Closed
daravi opened this issue Oct 16, 2020 · 12 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla cuda

Comments

@daravi
Copy link

daravi commented Oct 16, 2020

Bugzilla Link 47869
Resolution FIXED
Resolved on Oct 22, 2021 00:40
Version trunk
OS Linux
Blocks #51489
CC @carlosgalvezp,@daravi,@MaskRay,@Artem-B,@tstellar
Fixed by commit(s) 29e00b2 73daeb3

Extended Description

Trying to build TensorFlow with LLVM trunk, I arrived at this error

++ compilation of rule '//tensorflow/core/kernels/rnn:gru_ops_gpu' failed (Exit 1)
In file included from <built-in>:1:
In file included from /opt/llvm-master/lib/clang/12.0.0/include/__clang_cuda_runtime_wrapper.h:211:
In file included from /opt/llvm-master/bin/../include/c++/v1/string.h:60:
In file included from /usr/include/string.h:431:
In file included from /usr/include/strings.h:144:
/usr/include/bits/strings_fortified.h:23:8: error: exception specification in declaration does not match previous declaration
__NTH (bcopy (const void *__src, void *__dest, size_t __len))
       ^
/usr/include/strings.h:38:13: note: previous declaration is here
extern void bcopy (const void *__src, void *__dest, size_t __n)

Looking at those files I see that the macros are defined in /usr/include/sys/cdefs.h. This file defines __THROW, __NTH and __NTHNL with consistent exception specifier as they are used to declare and define the same functions. However __clang_cuda_runtime_wrapper.h re-defines only the __THROW macro:

// __THROW is redefined to be empty by device_functions_decls.h in CUDA. Clang's
// counterpart does not do it, so we need to make it empty here to keep
// following CUDA includes happy.
#undef __THROW
#define __THROW

I think the other two macros should also be cleared to avoid breaking the exception specifier consistency in libc. If you agree please let me know and I will be glad to contribute the fix.

@daravi
Copy link
Author

daravi commented Oct 16, 2020

assigned to @Artem-B

@Artem-B
Copy link
Member

Artem-B commented Oct 21, 2020

Could you attach complete clang command line and complete compiler output?
It would be even better if the output is from the compilation with additional -v flag.

Which CUDA version are you compiling with?
Do you use libstdc++ or libc++?
Which OS do you build on?

Tensorflow is probably the heaviest user of clang as the CUDA compiler and the header in question didn't change for quite a while. I'm not aware of this issue being triggered in our internal and public TF builds. My guess is that it's probably a corner case specific to your build. Or some recent change in the standard library headers that break the workarounds in the wrapper headers.

@daravi
Copy link
Author

daravi commented Dec 24, 2020

@​Artem Belevich Did you at all read my explanation of the issue? Which part of it do you disagree with?

It might be a while before I am back at this project and can give you all the details you have asked for. The relevant part of the compiler output is already in the previous message. I am using libc++. I think CUDA version was 6.1 if I remember correctly. The OS is CentOS 8.

@daravi
Copy link
Author

daravi commented Dec 24, 2020

[user@host]$ /lib/libc.so.6
GNU C Library (GNU libc) stable release version 2.28.
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 8.3.1 2019112 (Red Hat 8.3.1-5).
libc ABIs: UNIQUE IFUNC ABSOLUTE
For bug reporting instructions, please see:
http://www.gnu.org/software/libc/bugs.html.

@Artem-B
Copy link
Member

Artem-B commented Jan 6, 2021

I think the issue is being triggered by -D_FORTIFY_SOURCE.
If it's not used, preprocessed output only has one instance of bcopy() declaration.

So, on the surface, your suggestion to undefine __NTH* may work.
The problem is that this is another implementation detail that we'll need to make sure that it works everywhere. Testing it will be a challenge. We'll basically have to make the change as conservative as possible to minimize impact on the current users and roll back if someone complains.

Interestingly enough, recent CUDA versions no longer redefine __THROW.
We may need to make those re-definitions conditional on the CUDA version, too.

There's also a question of whether _FORTIFY_SOURCE is expected to work on the GPU side. My understanding that it requires some runtime support and there's no libraries for the GPU side. We may potentially need to disable _FORTIFY_SOURCE for GPU-side compilation, though it's probably a separate issue.

Feel free to send the patch for review and we'll discuss the details there.

@carlosgalvezp
Copy link
Contributor

I have encountered the same problem. Has a patch been published for review?

@Artem-B
Copy link
Member

Artem-B commented Sep 29, 2021

https://reviews.llvm.org/D110781 should fix it.

@Artem-B
Copy link
Member

Artem-B commented Oct 6, 2021

*** Bug #40735 has been marked as a duplicate of this bug. ***

@Artem-B
Copy link
Member

Artem-B commented Oct 19, 2021

We should include the fix in clang-13.0.1.

@Artem-B
Copy link
Member

Artem-B commented Oct 19, 2021

@tstellar
Copy link
Collaborator

Merged: 73daeb3

@tstellar
Copy link
Collaborator

mentioned in issue #51489

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla cuda
Projects
None yet
Development

No branches or pull requests

4 participants