Fix re-declarations of builtin functions with clang 10 #32837
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
omajid
changed the title
omajid
force-pushed
the
clang-10-llvm-throwdecl
branch
4 times, most recently
from
69abe8d to
365529b
Compare
omajid
changed the title
omajid
force-pushed
the
clang-10-llvm-throwdecl
branch
from
365529b to
dd5c8f0
Compare
|
Thanks to @serge-sans-paille for tracking down the root cause! |
omajid
force-pushed
the
clang-10-llvm-throwdecl
branch
from
dd5c8f0 to
4ead130
Compare
Clang commit 39aa8954a4846b317d3da2f0addfce8224b438de has moved
exception handling mismatches under the -fms-compatibility flag. This
breaks compilation of pal under clang 10 (and newer).
The compilation error looks like this:
In file included from .../pal/src/misc/tracepointprovider.cpp:19:
In file included from .../pal/src/include/pal/palinternal.h:620:
In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/stdlib.h:30:
/usr/include/stdlib.h:112:36: error: exception specification in declaration does not match previous declaration
__extension__ extern long long int atoll (const char *__nptr)
^
.../pal/inc/pal.h:4227:33: note: previous declaration is here
PALIMPORT long long int __cdecl atoll(const char *) THROW_DECL;
^
The simplest fix seems to be to make clang do the same thing as gcc and
define THROW_DECL as 'throw()'.
Testing via https://godbolt.org shows that even clang 3.3 compiles this
successfully without additional compiler options:
extern "C" long long atoll(char const*) throw();
#include <stdlib.h>
An alternative fix would be to use -fms-compatibility.
More details at https://bugzilla.redhat.com/show_bug.cgi?id=1807176
omajid
force-pushed
the
clang-10-llvm-throwdecl
branch
from
4ead130 to
4c8f9b2
Compare
omajid
changed the title
|
cc'ing some folks who last touched this part of the code, hoping to get a review: @janvorli @franksinankaya |
|
I just noticed that |
omajid
added a commit
to omajid/dotnet-coreclr
that referenced
this pull request
Mar 11, 2020
This fixes 3 different set of build issues are seen when compiling with clang 10. - Clang 10 fails to compile slist.h because the code contained is actually invalid. The assignment operator being used doesn't exist. This is a backport of dotnet/runtime#33096 - Clang 10 has moved exception-handling mismatches in function declarations under the -fms-compatibility flag (instead of the -fms-extensions flag). Our declarations of atoll and other similar functions are missing the exception declaration `throw()`. This mismatch in exception declarations makes clang 10 unable to build this code. Fix it by defining THROW_DECL as `throw()` which is supported at least as far back as clang 3.3. This is a backport of dotnet/runtime#32837 - Clang 10 has enabled (or made default?) the `misleading-indentation` warning which causes errors when trying to compile code in src/tools/metainfo/mdinfo.cpp. This is a backport of dotnet/runtime#33406
omajid
added a commit
to omajid/dotnet-coreclr
that referenced
this pull request
Mar 16, 2020
This fixes 3 different set of build issues are seen when compiling with clang 10. - Clang 10 fails to compile slist.h because the code contained is actually invalid. The assignment operator being used doesn't exist. This is a backport of dotnet/runtime#33096 - Clang 10 has moved exception-handling mismatches in function declarations under the -fms-compatibility flag (instead of the -fms-extensions flag). Our declarations of atoll and other similar functions are missing the exception declaration `throw()`. This mismatch in exception declarations makes clang 10 unable to build this code. Fix it by defining THROW_DECL as `throw()` which is supported at least as far back as clang 3.3. This is a backport of dotnet/runtime#32837 - Clang 10 has enabled additional warnings. Lets turn of -Werror globally in this release branch by making the `-ignorewarnings` switch to `./build.sh` be the default.
omajid
added a commit
to omajid/dotnet-coreclr
that referenced
this pull request
Mar 16, 2020
This fixes 3 different sets of build issues seen when compiling with Clang 10. - Clang 10 fails to compile slist.h because the code contained is actually invalid. The assignment operator being used doesn't exist. This is a backport of dotnet/runtime#33096 - Clang 10 has moved exception-handling mismatches in function declarations under the -fms-compatibility flag (instead of the -fms-extensions flag). Our declarations of atoll and other similar functions are missing the exception declaration `throw()`. This mismatch in exception declarations makes clang 10 unable to build this code. Fix it by defining THROW_DECL as `throw()` which is supported at least as far back as clang 3.3. This is a backport of dotnet/runtime#32837 - Clang 10 has enabled additional warnings. Lets turn of -Werror globally in this release branch by making the `-ignorewarnings` switch to `./build.sh` be the default.
Anipik
pushed a commit
to dotnet/coreclr
that referenced
this pull request
Mar 25, 2020
This fixes 3 different sets of build issues seen when compiling with Clang 10. - Clang 10 fails to compile slist.h because the code contained is actually invalid. The assignment operator being used doesn't exist. This is a backport of dotnet/runtime#33096 - Clang 10 has moved exception-handling mismatches in function declarations under the -fms-compatibility flag (instead of the -fms-extensions flag). Our declarations of atoll and other similar functions are missing the exception declaration `throw()`. This mismatch in exception declarations makes clang 10 unable to build this code. Fix it by defining THROW_DECL as `throw()` which is supported at least as far back as clang 3.3. This is a backport of dotnet/runtime#32837 - Clang 10 has enabled additional warnings. Lets turn of -Werror globally in this release branch by making the `-ignorewarnings` switch to `./build.sh` be the default.
omajid
added a commit
to omajid/dotnet-coreclr
that referenced
this pull request
Jun 2, 2020
This contains a grab bag of fixes to fix the build with clang 10. - dotnet#23075 Fix missing includes in coreclr/src/debug/createdump/ - dotnet/runtime#33096 SList::Init: add missing constructor - A subset of dotnet#22129 Just the parts that introduce the THROW_DECL macro in pal.h - dotnet/runtime#32837 This fixes THROW_DECL introduce in the previous PR to work with clang, which is required in clang 10. - One new change: In a significant divergance, this commits adds more THROW_DECL macros to all the math functions to address a ton of errors pointed out when building SOS: In file included from /home/omajid/devel/dotnet/coreclr/src/ToolBox/SOS/Strike/strike.cpp:116: In file included from /home/omajid/devel/dotnet/coreclr/src/vm/hillclimbing.h:19: In file included from /home/omajid/devel/dotnet/coreclr/src/inc/complex.h:16: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/math.h:36: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/cmath:45: In file included from /usr/include/math.h:290: /usr/include/bits/mathcalls.h:53:13: error: exception specification in declaration does not match previous declaration __MATHCALL (acos,, (_Mdouble_ __x)); ^ /home/omajid/devel/dotnet/coreclr/src/pal/inc/pal.h:4395:26: note: previous declaration is here PALIMPORT double __cdecl acos(double); ^ Then, to make sure the declarations and implementations match, it adds THROW_DECL to the definitions in src/pal/src/cruntime/math.cpp
omajid
added a commit
to omajid/dotnet-coreclr
that referenced
this pull request
Jun 2, 2020
This contains a grab bag of fixes to fix the build with clang 10. - dotnet#23075 Fix missing includes in coreclr/src/debug/createdump/ - dotnet/runtime#33096 SList::Init: add missing constructor - A subset of dotnet#22129 Just the parts that introduce the THROW_DECL macro in pal.h - dotnet/runtime#32837 This fixes THROW_DECL introduce in the previous PR to work with clang, which is required in clang 10. - One new change: In a significant divergance, this commits adds more THROW_DECL macros to all the math functions to address a ton of errors pointed out when building SOS: In file included from /home/omajid/devel/dotnet/coreclr/src/ToolBox/SOS/Strike/strike.cpp:116: In file included from /home/omajid/devel/dotnet/coreclr/src/vm/hillclimbing.h:19: In file included from /home/omajid/devel/dotnet/coreclr/src/inc/complex.h:16: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/math.h:36: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/cmath:45: In file included from /usr/include/math.h:290: /usr/include/bits/mathcalls.h:53:13: error: exception specification in declaration does not match previous declaration __MATHCALL (acos,, (_Mdouble_ __x)); ^ /home/omajid/devel/dotnet/coreclr/src/pal/inc/pal.h:4395:26: note: previous declaration is here PALIMPORT double __cdecl acos(double); ^ Then, to make sure the declarations and implementations match, it adds THROW_DECL to the definitions in src/pal/src/cruntime/math.cpp Co-authored-by: Mike McLaughlin <mikem@microsoft.com> Co-authored-by: Sinan Kaya <sinan.kaya@microsoft.com> Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
Anipik
pushed a commit
to dotnet/coreclr
that referenced
this pull request
Jun 11, 2020
This contains a grab bag of fixes to fix the build with clang 10. - #23075 Fix missing includes in coreclr/src/debug/createdump/ - dotnet/runtime#33096 SList::Init: add missing constructor - A subset of #22129 Just the parts that introduce the THROW_DECL macro in pal.h - dotnet/runtime#32837 This fixes THROW_DECL introduce in the previous PR to work with clang, which is required in clang 10. - One new change: In a significant divergance, this commits adds more THROW_DECL macros to all the math functions to address a ton of errors pointed out when building SOS: In file included from /home/omajid/devel/dotnet/coreclr/src/ToolBox/SOS/Strike/strike.cpp:116: In file included from /home/omajid/devel/dotnet/coreclr/src/vm/hillclimbing.h:19: In file included from /home/omajid/devel/dotnet/coreclr/src/inc/complex.h:16: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/math.h:36: In file included from /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/cmath:45: In file included from /usr/include/math.h:290: /usr/include/bits/mathcalls.h:53:13: error: exception specification in declaration does not match previous declaration __MATHCALL (acos,, (_Mdouble_ __x)); ^ /home/omajid/devel/dotnet/coreclr/src/pal/inc/pal.h:4395:26: note: previous declaration is here PALIMPORT double __cdecl acos(double); ^ Then, to make sure the declarations and implementations match, it adds THROW_DECL to the definitions in src/pal/src/cruntime/math.cpp Co-authored-by: Mike McLaughlin <mikem@microsoft.com> Co-authored-by: Sinan Kaya <sinan.kaya@microsoft.com> Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com> Co-authored-by: Mike McLaughlin <mikem@microsoft.com> Co-authored-by: Sinan Kaya <sinan.kaya@microsoft.com> Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clang commit 39aa8954a4846b317d3da2f0addfce8224b438de has moved exception handling mismatches under the
-fms-compatibilityflag. This breaks compilation of pal under clang 10 (and newer).The compilation error looks like this:
The simplest fix seems to be to make clang do the same thing as gcc and define
THROW_DECLasthrow().Testing via https://godbolt.org shows that even clang 3.3 compiles this successfully without additional compiler options:
An alternative fix would be to use
-fms-compatibility.More details at https://bugzilla.redhat.com/show_bug.cgi?id=1807176