[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. I'm curious why that file is declaring the builtin like that? The declaration doesn't seem like it is ever useful, builtins are available without any includes/etc as far back as I could go on all 3 compilers that I validated against. Additionally, I believe that the declaration is actually prohibited by the standard, since it is declaring an identifier that is reserved for the implementation. Is this a situation where the target project cannot be corrected? That line simply needs to be removed as far as I can tell. Repository: rL LLVM https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
arichardson added a comment. This broke the build of FreeBSD for me due to the declaration of __builtin_return_address(unsigned int) in https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Base.h#L1281: In file included from /exports/users/alr48/sources/freebsd-x86/sys/contrib/edk2/Include/Uefi.h:23: In file included from /exports/users/alr48/sources/freebsd-x86/sys/contrib/edk2/Include/Uefi/UefiBaseType.h:20: /exports/users/alr48/sources/freebsd-x86/sys/contrib/edk2/Include/Base.h:1231:10: error: cannot redeclare builtin function '__builtin_return_address' void * __builtin_return_address (unsigned int level); ^ /exports/users/alr48/sources/freebsd-x86/sys/contrib/edk2/Include/Base.h:1231:10: note: '__builtin_return_address' is a builtin with type 'void *(unsigned int)' Repository: rL LLVM https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330160: Limit types of builtins that can be redeclared.
(authored by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45383?vs=142695=142700#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45383
Files:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/Builtins.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Basic/Builtins.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/Sema/builtin-redecl.cpp
Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
return BuiltinMSVaListDecl;
}
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+ return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
assert(ObjCConstantStringType.isNull() &&
"'NSConstantString' type already set!");
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@
if (Old->isInvalidDecl())
return true;
+ // Disallow redeclaration of some builtins.
+ if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+ }
+
diag::kind PrevDiag;
SourceLocation OldLocation;
std::tie(PrevDiag, OldLocation) =
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
Match = *I;
return Ovl_Match;
}
+
+ // Builtins that have custom typechecking or have a reference should
+ // not be overloadable or redeclarable.
+ if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+ }
} else if (isa(OldD) || isa(OldD)) {
// We can overload with these, which can show up when doing
// redeclaration checks for UsingDecls.
Index: cfe/trunk/lib/Basic/Builtins.cpp
===
--- cfe/trunk/lib/Basic/Builtins.cpp
+++ cfe/trunk/lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@
bool ) {
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+ return ID == Builtin::NotBuiltin ||
+ ID == Builtin::BI__va_start ||
+ (!hasReferenceArgsOrResult(ID) &&
+ !hasCustomTypechecking(ID));
+}
Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
return getTypeDeclType(getBuiltinMSVaListDecl());
}
+ /// Return whether a declaration to a builtin is allowed to be
+ /// overloaded/redeclared.
+ bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
/// \brief Return a type with additional \c const, \c volatile, or
/// \c restrict qualifiers.
QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: cfe/trunk/include/clang/Basic/Builtins.h
===
--- cfe/trunk/include/clang/Basic/Builtins.h
+++ cfe/trunk/include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
return strchr(getRecord(ID).Type, '*') != nullptr;
}
+ /// \brief Return true if this builtin has a result or any arguments which are
+ /// reference types.
+ bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+ strchr(getRecord(ID).Type, 'A') != nullptr;
+ }
+
/// \brief Completely forget that the given ID was ever considered a builtin,
/// e.g., because the user provided a conflicting signature.
void forgetBuiltin(unsigned ID, IdentifierTable );
@@ -212,6 +219,10 @@
/// prefix.
static bool isBuiltinFunc(const char *Name);
+ /// Returns true if this is a builtin that can be redeclared. Returns true
+ /// for non-builtins.
+ bool canBeRedeclared(unsigned ID) const;
+
private:
const Info (unsigned ID) const;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 142695.
erichkeane added a comment.
Revert unnecessary test changes... :/
https://reviews.llvm.org/D45383
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ASTContext.cpp
lib/Basic/Builtins.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaOverload.cpp
test/Sema/builtin-redecl.cpp
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
return BuiltinMSVaListDecl;
}
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+ return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
assert(ObjCConstantStringType.isNull() &&
"'NSConstantString' type already set!");
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
Match = *I;
return Ovl_Match;
}
+
+ // Builtins that have custom typechecking or have a reference should
+ // not be overloadable or redeclarable.
+ if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+ }
} else if (isa(OldD) || isa(OldD)) {
// We can overload with these, which can show up when doing
// redeclaration checks for UsingDecls.
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@
if (Old->isInvalidDecl())
return true;
+ // Disallow redeclaration of some builtins.
+ if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+ }
+
diag::kind PrevDiag;
SourceLocation OldLocation;
std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@
bool ) {
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+ return ID == Builtin::NotBuiltin ||
+ ID == Builtin::BI__va_start ||
+ (!hasReferenceArgsOrResult(ID) &&
+ !hasCustomTypechecking(ID));
+}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
return getTypeDeclType(getBuiltinMSVaListDecl());
}
+ /// Return whether a declaration to a builtin is allowed to be
+ /// overloaded/redeclared.
+ bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
/// \brief Return a type with additional \c const, \c volatile, or
/// \c restrict qualifiers.
QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
"incompatible redeclaration of library function %0">,
InGroup>;
def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
return strchr(getRecord(ID).Type, '*') != nullptr;
}
+ /// \brief Return true if this builtin has a result or any arguments which are
+ /// reference types.
+ bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+ strchr(getRecord(ID).Type, 'A') != nullptr;
+ }
+
/// \brief Completely forget that the given ID was ever considered a builtin,
/// e.g., because the user provided a conflicting signature.
void forgetBuiltin(unsigned ID, IdentifierTable );
@@ -212,6 +219,10 @@
/// prefix.
static bool isBuiltinFunc(const char *Name);
+ /// Returns true if this is a builtin that can be redeclared. Returns true
+ /// for non-builtins.
+ bool canBeRedeclared(unsigned ID) const;
+
private:
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. It looks like you didn't fix the tests? https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 142685.
erichkeane added a comment.
Added the exception for MSVC's __va_start. __va_start is a normal function on
Linux, so it isn't an error on non-windows either.
https://reviews.llvm.org/D45383
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ASTContext.cpp
lib/Basic/Builtins.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaOverload.cpp
test/Sema/MicrosoftExtensions.c
test/Sema/builtin-redecl.cpp
test/SemaCXX/microsoft-varargs.cpp
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
return BuiltinMSVaListDecl;
}
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+ return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
assert(ObjCConstantStringType.isNull() &&
"'NSConstantString' type already set!");
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
Match = *I;
return Ovl_Match;
}
+
+ // Builtins that have custom typechecking or have a reference should
+ // not be overloadable or redeclarable.
+ if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+ }
} else if (isa(OldD) || isa(OldD)) {
// We can overload with these, which can show up when doing
// redeclaration checks for UsingDecls.
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@
if (Old->isInvalidDecl())
return true;
+ // Disallow redeclaration of some builtins.
+ if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+ }
+
diag::kind PrevDiag;
SourceLocation OldLocation;
std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@
bool ) {
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+ return ID == Builtin::NotBuiltin ||
+ ID == Builtin::BI__va_start ||
+ (!hasReferenceArgsOrResult(ID) &&
+ !hasCustomTypechecking(ID));
+}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
return getTypeDeclType(getBuiltinMSVaListDecl());
}
+ /// Return whether a declaration to a builtin is allowed to be
+ /// overloaded/redeclared.
+ bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
/// \brief Return a type with additional \c const, \c volatile, or
/// \c restrict qualifiers.
QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
"incompatible redeclaration of library function %0">,
InGroup>;
def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
return strchr(getRecord(ID).Type, '*') != nullptr;
}
+ /// \brief Return true if this builtin has a result or any arguments which are
+ /// reference types.
+ bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+ strchr(getRecord(ID).Type, 'A') != nullptr;
+ }
+
/// \brief Completely forget that the given ID was ever considered a builtin,
/// e.g., because the user provided a conflicting signature.
void forgetBuiltin(unsigned ID, IdentifierTable );
@@ -212,6 +219,10 @@
/// prefix.
static bool isBuiltinFunc(const char *Name);
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1069057, @efriedma wrote: > I don't see an `A` in `LANGBUILTIN(__va_start, "vc**.", "nt", > ALL_MS_LANGUAGES)` Ah, you're right! I was confusing it with LIBBUILTIN(va_start, "vA.", "fn", "stdarg.h", ALL_LANGUAGES) and BUILTIN(__builtin_va_start, "vA.", "nt"). It seems that the MS builtins dont' actually use 'A'. I'll implement your suggestion, thanks for your patience :) https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. I don't see an `A` in `LANGBUILTIN(__va_start, "vc**.", "nt", ALL_MS_LANGUAGES)` https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1069049, @efriedma wrote: > We can could add an exception to the "don't allow redeclarations with custom > type-checking" rule to specifically allow redeclaring `__va_start`. The > general rule is that we don't want user code redeclaring them because they > don't have a specific signature, so our internal representation could change > between releases. But `__va_start` has a specific fixed signature from the > Microsoft SDK headers, so it should be fine to allow redeclaring it without a > warning. That still doesn't fix the bug, since redeclaring __va_start in 'C' mode when va_list is a reference type causes the assert. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added a comment. We can could add an exception to the "don't allow redeclarations with custom type-checking" rule to specifically allow redeclaring `__va_start`. The general rule is that we don't want user code redeclaring them because they don't have a specific signature, so our internal representation could change between releases. But `__va_start` has a specific fixed signature from the Microsoft SDK headers, so it should be fine to allow redeclaring it without a warning. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1068085, @compnerd wrote: > Snipping bits from `va_defs.h`: > > > #elif defined _M_ARM64 > > void __cdecl __va_start(va_list*, ...); > > #define __crt_va_start_a(ap,v) ((void)(__va_start(, _ADDRESSOF(v), > _SLOTSIZEOF(v), __alignof(v), _ADDRESSOF(v > ... > > #elif defined _M_X64 > > void __cdecl __va_start(va_list* , ...); > > #define __crt_va_start_a(ap, x) ((void)(__va_start(, x))) > > ... > > > This looks like a declaration to me. Although, this is in system headers, so > maybe we can ignore it by means of the system header suppression? The minor > problem with that is that clang-cl (and the clang driver with the windows > triple) do not support `-isystem` or `-isysroot` or `--sysroot` arguments. I > suppose that as long as we expose the cc1 option (I imagine that clang-cl > will pass the system paths appropriately), that is one option. Ah, thank you! I couldn't find that when I looked after your last message, perhaps my grep skills are lacking. Unfortunately, allowing this in headers won't fix the actual bug, it'll just let it continue to crash on these headers. I'm kind of out of ideas here... @efriedma : Ideas? https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
compnerd added a comment. Snipping bits from `va_defs.h`: #elif defined _M_ARM64 void __cdecl __va_start(va_list*, ...); #define __crt_va_start_a(ap,v) ((void)(__va_start(, _ADDRESSOF(v), _SLOTSIZEOF(v), __alignof(v), _ADDRESSOF(v ... #elif defined _M_X64 void __cdecl __va_start(va_list* , ...); #define __crt_va_start_a(ap, x) ((void)(__va_start(, x))) ... This looks like a declaration to me. Although, this is in system headers, so maybe we can ignore it by means of the system header suppression? The minor problem with that is that clang-cl (and the clang driver with the windows triple) do not support `-isystem` or `-isysroot` or `--sysroot` arguments. I suppose that as long as we expose the cc1 option (I imagine that clang-cl will pass the system paths appropriately), that is one option. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane added a comment. In https://reviews.llvm.org/D45383#1065999, @compnerd wrote: > I know that the Windows SDK definitely declares the `__va_start` function. > Did you try building something like swift against the Windows SDK with this > change? Are we sure it declares it, and not just #define's it? If it actually declares it, there is no way it can currently build in certain C modes due to the issue I'd mentioned previously. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
compnerd added a comment. I know that the Windows SDK definitely declares the `__va_start` function. Did you try building something like swift against the Windows SDK with this change? https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 141900.
erichkeane marked an inline comment as done.
erichkeane added a comment.
Restrict overloads as well.
https://reviews.llvm.org/D45383
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ASTContext.cpp
lib/Basic/Builtins.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaOverload.cpp
test/Sema/MicrosoftExtensions.c
test/Sema/builtin-redecl.cpp
test/SemaCXX/microsoft-varargs.cpp
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@
return BuiltinMSVaListDecl;
}
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+ return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
assert(ObjCConstantStringType.isNull() &&
"'NSConstantString' type already set!");
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@
Match = *I;
return Ovl_Match;
}
+
+ // Builtins that have custom typechecking or have a reference should
+ // not be overloadable or redeclarable.
+ if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+Match = *I;
+return Ovl_NonFunction;
+ }
} else if (isa(OldD) || isa(OldD)) {
// We can overload with these, which can show up when doing
// redeclaration checks for UsingDecls.
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3014,6 +3014,14 @@
if (Old->isInvalidDecl())
return true;
+ // Disallow redeclaration of some builtins.
+ if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+ }
+
diag::kind PrevDiag;
SourceLocation OldLocation;
std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,9 @@
bool ) {
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+ return ID == Builtin::NotBuiltin ||
+ (!hasReferenceArgsOrResult(ID) &&
+ !hasCustomTypechecking(ID));
+}
Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@
return getTypeDeclType(getBuiltinMSVaListDecl());
}
+ /// Return whether a declaration to a builtin is allowed to be
+ /// overloaded/redeclared.
+ bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
/// \brief Return a type with additional \c const, \c volatile, or
/// \c restrict qualifiers.
QualType getCVRQualifiedType(QualType T, unsigned CVR) const {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
"incompatible redeclaration of library function %0">,
InGroup>;
def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
return strchr(getRecord(ID).Type, '*') != nullptr;
}
+ /// \brief Return true if this builtin has a result or any arguments which are
+ /// reference types.
+ bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+ strchr(getRecord(ID).Type, 'A') != nullptr;
+ }
+
/// \brief Completely forget that the given ID was ever considered a builtin,
/// e.g., because the user provided a conflicting signature.
void forgetBuiltin(unsigned ID, IdentifierTable );
@@ -212,6 +219,10 @@
/// prefix.
static bool isBuiltinFunc(const char *Name);
+ /// Returns true if this is a builtin that can be redeclared. Returns true
+ /// for
[PATCH] D45383: Limit types of builtins that can be redeclared.
efriedma added inline comments. Comment at: test/Sema/builtin-redecl.cpp:9 +// Overloading a builtin is acceptable in C++. +void __builtin_va_copy(double d); +#endif We don't want to allow this; in particular, for builtins with custom type-checking, we have no way to correctly resolve which overload to call. Probably Sema::CheckOverload should be checking canBeRedeclared. https://reviews.llvm.org/D45383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45383: Limit types of builtins that can be redeclared.
erichkeane updated this revision to Diff 141876.
erichkeane retitled this revision from "Strip reference from a va_list object
in C when merging parameter types." to "Limit types of builtins that can be
redeclared.".
erichkeane edited the summary of this revision.
erichkeane added a comment.
Eli's suggestion for implementation.
https://reviews.llvm.org/D45383
Files:
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Basic/Builtins.cpp
lib/Sema/SemaDecl.cpp
test/Sema/MicrosoftExtensions.c
test/Sema/builtin-redecl.cpp
test/SemaCXX/microsoft-varargs.cpp
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -3014,6 +3014,14 @@
if (Old->isInvalidDecl())
return true;
+ // Disallow redeclaration of some builtins.
+ if (!getASTContext().BuiltinInfo.canBeRedeclared(Old->getBuiltinID())) {
+Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+<< Old << Old->getType();
+return true;
+ }
+
diag::kind PrevDiag;
SourceLocation OldLocation;
std::tie(PrevDiag, OldLocation) =
Index: lib/Basic/Builtins.cpp
===
--- lib/Basic/Builtins.cpp
+++ lib/Basic/Builtins.cpp
@@ -139,3 +139,9 @@
bool ) {
return isLike(ID, FormatIdx, HasVAListArg, "sS");
}
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+ return ID == Builtin::NotBuiltin ||
+ (!hasReferenceArgsOrResult(ID) &&
+ !hasCustomTypechecking(ID));
+}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@
"incompatible redeclaration of library function %0">,
InGroup>;
def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;
Index: include/clang/Basic/Builtins.h
===
--- include/clang/Basic/Builtins.h
+++ include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@
return strchr(getRecord(ID).Type, '*') != nullptr;
}
+ /// \brief Return true if this builtin has a result or any arguments which are
+ /// reference types.
+ bool hasReferenceArgsOrResult(unsigned ID) const {
+return strchr(getRecord(ID).Type, '&') != nullptr ||
+ strchr(getRecord(ID).Type, 'A') != nullptr;
+ }
+
/// \brief Completely forget that the given ID was ever considered a builtin,
/// e.g., because the user provided a conflicting signature.
void forgetBuiltin(unsigned ID, IdentifierTable );
@@ -212,6 +219,10 @@
/// prefix.
static bool isBuiltinFunc(const char *Name);
+ /// Returns true if this is a builtin that can be redeclared. Returns true
+ /// for non-builtins.
+ bool canBeRedeclared(unsigned ID) const;
+
private:
const Info (unsigned ID) const;
Index: test/SemaCXX/microsoft-varargs.cpp
===
--- test/SemaCXX/microsoft-varargs.cpp
+++ test/SemaCXX/microsoft-varargs.cpp
@@ -3,7 +3,6 @@
extern "C" {
typedef char * va_list;
-void __va_start(va_list *, ...);
}
int test___va_start(int i, ...) {
Index: test/Sema/MicrosoftExtensions.c
===
--- test/Sema/MicrosoftExtensions.c
+++ test/Sema/MicrosoftExtensions.c
@@ -166,7 +166,6 @@
T __ptr32 wrong10; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
typedef char *my_va_list;
-void __va_start(my_va_list *ap, ...); // expected-note {{passing argument to parameter 'ap' here}}
void vmyprintf(const char *f, my_va_list ap);
void myprintf(const char *f, ...) {
my_va_list ap;
Index: test/Sema/builtin-redecl.cpp
===
--- test/Sema/builtin-redecl.cpp
+++ test/Sema/builtin-redecl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -fsyntax-only -verify -x c
+
+// Redeclaring library builtins is OK.
+void exit(int);
+
+#if __cplusplus
+// Overloading a builtin is acceptable in C++.
+void __builtin_va_copy(double d);
+#endif
+
+// expected-error@+2 {{cannot redeclare builtin function '__builtin_va_end'}}
+// expected-note@+1 {{'__builtin_va_end' is a builtin with type}}
+void __builtin_va_end(__builtin_va_list);
