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][Sema] Don't issue -Wcast-function-type-mismatch for enums with a matching underlying type #87793

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

tambry
Copy link
Contributor

@tambry tambry commented Apr 5, 2024

Enums are passed as their underlying integral type so they're ABI compatible if the size matches.
Useful with C APIs that pass user-controlled values to callbacks that can be made type safe by using enumerations (e.g. GStreamer).

Discovered internally in some code after 999d4f8.

@tambry tambry added clang Clang issues not falling into any other category clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Apr 5, 2024
@tambry tambry self-assigned this Apr 5, 2024
@llvmbot llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Apr 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 5, 2024

@llvm/pr-subscribers-clang

Author: Raul Tambre (tambry)

Changes

Enums are passed as their underlying integral type so they're ABI compatible if the size matches.
Useful with C APIs that pass user-controlled values to callbacks that can be made type safe by using enumerations (e.g. GStreamer).

Discovered internally in some code after 999d4f8.


Full diff: https://github.com/llvm/llvm-project/pull/87793.diff

5 Files Affected:

  • (modified) clang/lib/Sema/SemaCast.cpp (+4-3)
  • (modified) clang/test/Sema/warn-cast-function-type-strict.c (+4)
  • (modified) clang/test/Sema/warn-cast-function-type.c (+4)
  • (modified) clang/test/SemaCXX/warn-cast-function-type-strict.cpp (+4)
  • (modified) clang/test/SemaCXX/warn-cast-function-type.cpp (+4)
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 9d85568d97b2d2..b93724a85999be 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1083,9 +1083,10 @@ static bool argTypeIsABIEquivalent(QualType SrcType, QualType DestType,
     return true;
 
   // Allow integral type mismatch if their size are equal.
-  if (SrcType->isIntegralType(Context) && DestType->isIntegralType(Context))
-    if (Context.getTypeInfoInChars(SrcType).Width ==
-        Context.getTypeInfoInChars(DestType).Width)
+  if ((SrcType->isIntegralType(Context) || SrcType->isEnumeralType()) &&
+      (DestType->isIntegralType(Context) || DestType->isEnumeralType()))
+    if (Context.getTypeSizeInChars(SrcType) ==
+        Context.getTypeSizeInChars(DestType))
       return true;
 
   return Context.hasSameUnqualifiedType(SrcType, DestType);
diff --git a/clang/test/Sema/warn-cast-function-type-strict.c b/clang/test/Sema/warn-cast-function-type-strict.c
index b0a70cf324b711..b21365cdd30a57 100644
--- a/clang/test/Sema/warn-cast-function-type-strict.c
+++ b/clang/test/Sema/warn-cast-function-type-strict.c
@@ -29,8 +29,12 @@ f8 *h;
 f9 *i;
 f10 *j;
 
+enum E : long;
+int efunc(enum E);
+
 void foo(void) {
   a = (f1 *)x;
+  a = (f1 *)efunc; // strict-warning {{cast from 'int (*)(enum E)' to 'f1 *' (aka 'int (*)(long)') converts to incompatible function type}}
   b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
   c = (f3 *)x; /* strict-warning {{cast from 'int (*)(long)' to 'f3 *' (aka 'int (*)()') converts to incompatible function type}} */
   d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)()') converts to incompatible function type}} */
diff --git a/clang/test/Sema/warn-cast-function-type.c b/clang/test/Sema/warn-cast-function-type.c
index 09d169026b1c86..969a62043791f4 100644
--- a/clang/test/Sema/warn-cast-function-type.c
+++ b/clang/test/Sema/warn-cast-function-type.c
@@ -19,8 +19,12 @@ f5 *e;
 f6 *f;
 f7 *g;
 
+enum E : long;
+int efunc(enum E);
+
 void foo(void) {
   a = (f1 *)x;
+  a = (f1 *)efunc; // enum is just type system sugar, still passed as a long.
   b = (f2 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}} */
   c = (f3 *)x;
   d = (f4 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f4 *' (aka 'void (*)()') converts to incompatible function type}} */
diff --git a/clang/test/SemaCXX/warn-cast-function-type-strict.cpp b/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
index 8887b3c4c5d535..c49b0ac4b26a23 100644
--- a/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
+++ b/clang/test/SemaCXX/warn-cast-function-type-strict.cpp
@@ -29,8 +29,12 @@ struct S
 
 typedef void (S::*mf)(int);
 
+enum E : long;
+int efunc(E);
+
 void foo() {
   a = (f1 *)x;
+  a = (f1 *)efunc; // strict-warning {{cast from 'int (*)(E)' to 'f1 *' (aka 'int (*)(long)') converts to incompatible function type}}
   b = (f2 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
   b = reinterpret_cast<f2 *>(x); // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
   c = (f3 *)x; // strict-warning {{cast from 'int (*)(long)' to 'f3 *' (aka 'int (*)(...)') converts to incompatible function type}}
diff --git a/clang/test/SemaCXX/warn-cast-function-type.cpp b/clang/test/SemaCXX/warn-cast-function-type.cpp
index db2ee030fcbfc9..467fcd6abc2482 100644
--- a/clang/test/SemaCXX/warn-cast-function-type.cpp
+++ b/clang/test/SemaCXX/warn-cast-function-type.cpp
@@ -28,8 +28,12 @@ struct S
 
 typedef void (S::*mf)(int);
 
+enum E : long;
+int efunc(E);
+
 void foo() {
   a = (f1 *)x;
+  a = (f1 *)efunc; // enum is just type system sugar, still passed as a long.
   b = (f2 *)x; // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
   b = reinterpret_cast<f2 *>(x); // expected-warning {{cast from 'int (*)(long)' to 'f2 *' (aka 'int (*)(void *)') converts to incompatible function type}}
   c = (f3 *)x;

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delayed review, this fell off my radar. No release note needed because this is fixing an issue introduced in this release. LGTM with a small testing request.

clang/test/Sema/warn-cast-function-type.c Show resolved Hide resolved
…h a matching underlying type

Enums are passed as their underlying integral type so they're ABI compatible if the size matches.
Useful with C APIs that pass user-controlled values to callbacks that can be made type safe by using enumerations (e.g. GStreamer).
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Do you need someone to land this on your behalf?

@tambry
Copy link
Contributor Author

tambry commented Jun 3, 2024

I'm able to land myself. 🙂
Waiting for the CI to go green.

@tambry tambry merged commit 2bc098b into llvm:main Jun 3, 2024
5 of 7 checks passed
@tambry tambry deleted the cast_mismatch branch June 3, 2024 17:57
vedantparanjape-amd pushed a commit to vedantparanjape-amd/llvm-project that referenced this pull request Jun 7, 2024
…h a matching underlying type (llvm#87793)

Enums are passed as their underlying integral type so they're ABI compatible if the size matches.
Useful with C APIs that pass user-controlled values to callbacks that can be made type safe by using enumerations (e.g. GStreamer).

Discovered internally in some code after 999d4f8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants