tambre created this revision.
tambre added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We didn't mark them previously as non-builtin in non-global scope if we
determined they were incompatible redeclarations.
This caused problems down the line with them still being treated as builtins
and having attributes added and the like.
Fix this by marking them non-builtin in any case. Handle redeclarations of such
redeclarations as if they were redeclarations of the builtins by checking if
the previous redeclaration was reverted from being a builtin.
Fixes PR45410.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77491
Files:
clang/include/clang/Basic/IdentifierTable.h
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGen/builtin-redeclaration.c
Index: clang/test/CodeGen/builtin-redeclaration.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/builtin-redeclaration.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+// PR45410
+// Ensure we mark local extern redeclarations with a different type as
non-builtin.
+void non_builtin() {
+ extern float exp();
+ exp(); // Will crash due to wrong number of arguments if this calls the
builtin.
+}
+
+// PR45410
+// We mark exp() builtin as const with -fno-math-errno (default).
+// We mustn't do that for extern redeclarations of builtins where the type
differs.
+float attribute() {
+ extern float exp();
+ return exp(1);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3756,24 +3756,21 @@
// If the previous declaration was an implicitly-generated builtin
// declaration, then at the very least we should use a specialized note.
unsigned BuiltinID;
- if (Old->isImplicit() && (BuiltinID = Old->getBuiltinID())) {
+ bool RevertedBuiltin{Old->getIdentifier()->hasRevertedBuiltin()};
+ if (Old->isImplicit() &&
+ ((BuiltinID = Old->getBuiltinID()) || RevertedBuiltin)) {
// If it's actually a library-defined builtin function like 'malloc'
// or 'printf', just warn about the incompatible redeclaration.
- if (Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
+ // Also warn if it's a redeclaration of a builtin redeclaration. We know if
+ // it is if the previous declaration is a reverted builtin.
+ if (RevertedBuiltin ||
+ Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
Diag(New->getLocation(), diag::warn_redecl_library_builtin) << New;
Diag(OldLocation, diag::note_previous_builtin_declaration)
<< Old << Old->getType();
- // If this is a global redeclaration, just forget hereafter
- // about the "builtin-ness" of the function.
- //
- // Doing this for local extern declarations is problematic. If
- // the builtin declaration remains visible, a second invalid
- // local declaration will produce a hard error; if it doesn't
- // remain visible, a single bogus local redeclaration (which is
- // actually only a warning) could break all the downstream code.
- if (!New->getLexicalDeclContext()->isFunctionOrMethod())
- New->getIdentifier()->revertBuiltin();
+ // Forget hereafter about the "builtin-ness" of the function.
+ New->getIdentifier()->revertBuiltin();
return false;
}
Index: clang/include/clang/Basic/IdentifierTable.h
===================================================================
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -223,7 +223,7 @@
}
void setObjCKeywordID(tok::ObjCKeywordKind ID) { ObjCOrBuiltinID = ID; }
- /// True if setNotBuiltin() was called.
+ /// True if revertBuiltin() was called.
bool hasRevertedBuiltin() const {
return ObjCOrBuiltinID == tok::NUM_OBJC_KEYWORDS;
}
Index: clang/test/CodeGen/builtin-redeclaration.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/builtin-redeclaration.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+// PR45410
+// Ensure we mark local extern redeclarations with a different type as non-builtin.
+void non_builtin() {
+ extern float exp();
+ exp(); // Will crash due to wrong number of arguments if this calls the builtin.
+}
+
+// PR45410
+// We mark exp() builtin as const with -fno-math-errno (default).
+// We mustn't do that for extern redeclarations of builtins where the type differs.
+float attribute() {
+ extern float exp();
+ return exp(1);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3756,24 +3756,21 @@
// If the previous declaration was an implicitly-generated builtin
// declaration, then at the very least we should use a specialized note.
unsigned BuiltinID;
- if (Old->isImplicit() && (BuiltinID = Old->getBuiltinID())) {
+ bool RevertedBuiltin{Old->getIdentifier()->hasRevertedBuiltin()};
+ if (Old->isImplicit() &&
+ ((BuiltinID = Old->getBuiltinID()) || RevertedBuiltin)) {
// If it's actually a library-defined builtin function like 'malloc'
// or 'printf', just warn about the incompatible redeclaration.
- if (Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
+ // Also warn if it's a redeclaration of a builtin redeclaration. We know if
+ // it is if the previous declaration is a reverted builtin.
+ if (RevertedBuiltin ||
+ Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID)) {
Diag(New->getLocation(), diag::warn_redecl_library_builtin) << New;
Diag(OldLocation, diag::note_previous_builtin_declaration)
<< Old << Old->getType();
- // If this is a global redeclaration, just forget hereafter
- // about the "builtin-ness" of the function.
- //
- // Doing this for local extern declarations is problematic. If
- // the builtin declaration remains visible, a second invalid
- // local declaration will produce a hard error; if it doesn't
- // remain visible, a single bogus local redeclaration (which is
- // actually only a warning) could break all the downstream code.
- if (!New->getLexicalDeclContext()->isFunctionOrMethod())
- New->getIdentifier()->revertBuiltin();
+ // Forget hereafter about the "builtin-ness" of the function.
+ New->getIdentifier()->revertBuiltin();
return false;
}
Index: clang/include/clang/Basic/IdentifierTable.h
===================================================================
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -223,7 +223,7 @@
}
void setObjCKeywordID(tok::ObjCKeywordKind ID) { ObjCOrBuiltinID = ID; }
- /// True if setNotBuiltin() was called.
+ /// True if revertBuiltin() was called.
bool hasRevertedBuiltin() const {
return ObjCOrBuiltinID == tok::NUM_OBJC_KEYWORDS;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
