Skip to content

[HLSL] Disallow virtual inheritance and functions #127346

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

Merged
merged 2 commits into from
Mar 9, 2025

Conversation

llvm-beanz
Copy link
Collaborator

This PR disallows virtual inheritance and virtual functions in HLSL.

This PR disallows virtual inheritance and virtual functions in HLSL.
@llvm-beanz llvm-beanz requested review from spall and V-FEXrt February 15, 2025 20:36
@llvm-beanz llvm-beanz added the HLSL HLSL Language Support label Feb 15, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Chris B (llvm-beanz)

Changes

This PR disallows virtual inheritance and virtual functions in HLSL.


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

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+6-1)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+3)
  • (added) clang/test/SemaHLSL/Language/NoVirtual.hlsl (+14)
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c513dab810d1f..bec3b5d48fd51 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1817,5 +1817,7 @@ def ext_hlsl_access_specifiers : ExtWarn<
   InGroup<HLSLExtension>;
 def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expected 'x', 'y', 'z', or 'w'">;
 def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">;
+def err_hlsl_virtual_function : Error<"virtual functions are unsupported in HLSL">;
+def err_hlsl_virtual_inheritance : Error<"virtual inheritance is unsupported in HLSL">;
 
 } // end of Parser diagnostics
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7ae136af47391..dfa2dbf5ab61f 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4411,7 +4411,12 @@ void Parser::ParseDeclarationSpecifiers(
         DiagID = diag::err_openclcxx_virtual_function;
         PrevSpec = Tok.getIdentifierInfo()->getNameStart();
         isInvalid = true;
-      } else {
+      } else if (getLangOpts().HLSL) {
+        DiagID = diag::err_hlsl_virtual_function;
+        PrevSpec = Tok.getIdentifierInfo()->getNameStart();
+        isInvalid = true;
+      }
+      else {
         isInvalid = DS.setFunctionSpecVirtual(Loc, PrevSpec, DiagID);
       }
       break;
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 43db715ac6d70..dbc6d5f7c43a3 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2491,6 +2491,9 @@ BaseResult Parser::ParseBaseSpecifier(Decl *ClassDecl) {
     IsVirtual = true;
   }
 
+  if (getLangOpts().HLSL && IsVirtual)
+    Diag(Tok.getLocation(), diag::err_hlsl_virtual_inheritance);
+
   CheckMisplacedCXX11Attribute(Attributes, StartLoc);
 
   // Parse the class-name.
diff --git a/clang/test/SemaHLSL/Language/NoVirtual.hlsl b/clang/test/SemaHLSL/Language/NoVirtual.hlsl
new file mode 100644
index 0000000000000..8d61bde7d836e
--- /dev/null
+++ b/clang/test/SemaHLSL/Language/NoVirtual.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -verify %s
+
+struct Base {
+  int X;
+  void MemberFunction();  // valid
+  virtual void MemberFunction2(); // expected-error{{virtual functions are unsupported in HLSL}}
+};
+
+struct Derived : virtual Base { // expected-error{{virtual inheritance is unsupported in HLSL}}
+  int Y;
+
+  void MemberFunction2() override; // expected-error{{only virtual member functions can be marked 'override'}}
+};
+

Copy link

github-actions bot commented Feb 15, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -1817,5 +1817,9 @@ def ext_hlsl_access_specifiers : ExtWarn<
InGroup<HLSLExtension>;
def err_hlsl_unsupported_component : Error<"invalid component '%0' used; expected 'x', 'y', 'z', or 'w'">;
def err_hlsl_packoffset_invalid_reg : Error<"invalid resource class specifier '%0' for packoffset, expected 'c'">;
def err_hlsl_virtual_function
Copy link
Contributor

Choose a reason for hiding this comment

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

could you make this 1 diagnostic with a selector?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately sharing the diagnostic would require a significant reworking of ParseDecl. The diagnostic for virtual functions is not emitted directly at the place where we encounter the keyword, instead we set the decl to invalid and provide the diagnostic ID. There isn't a way currently to also provide diagnostic arguments.

@llvm-beanz llvm-beanz merged commit a7d5b3f into llvm:main Mar 9, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 9, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-quick running on linaro-clang-aarch64-quick while building clang at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/13464

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'lit :: googletest-timeout.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 9
not env -u FILECHECK_OPTS "/usr/bin/python3.10" /home/tcwg-buildbot/worker/clang-aarch64-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout    --param gtest_filter=InfiniteLoopSubTest --timeout=1 > /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out
# executed command: not env -u FILECHECK_OPTS /usr/bin/python3.10 /home/tcwg-buildbot/worker/clang-aarch64-quick/llvm/llvm/utils/lit/lit.py -j1 --order=lexical -v Inputs/googletest-timeout --param gtest_filter=InfiniteLoopSubTest --timeout=1
# .---command stderr------------
# | lit.py: /home/tcwg-buildbot/worker/clang-aarch64-quick/llvm/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# `-----------------------------
# RUN: at line 11
FileCheck --check-prefix=CHECK-INF < /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/Output/googletest-timeout.py.tmp.cmd.out /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/googletest-timeout.py
# executed command: FileCheck --check-prefix=CHECK-INF /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/googletest-timeout.py
# .---command stderr------------
# | /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/googletest-timeout.py:34:14: error: CHECK-INF: expected string not found in input
# | # CHECK-INF: Timed Out: 1
# |              ^
# | <stdin>:13:29: note: scanning from here
# | Reached timeout of 1 seconds
# |                             ^
# | <stdin>:37:2: note: possible intended match here
# |  Timed Out: 2 (100.00%)
# |  ^
# | 
# | Input file: <stdin>
# | Check file: /home/tcwg-buildbot/worker/clang-aarch64-quick/stage1/utils/lit/tests/googletest-timeout.py
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |             .
# |             .
# |             .
# |             8:  
# |             9:  
# |            10: -- 
# |            11: exit: -9 
# |            12: -- 
# |            13: Reached timeout of 1 seconds 
# | check:34'0                                 X error: no match found
# |            14: ******************** 
# | check:34'0     ~~~~~~~~~~~~~~~~~~~~~
# |            15: TIMEOUT: googletest-timeout :: DummySubDir/OneTest.py/1/2 (2 of 2) 
# | check:34'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            16: ******************** TEST 'googletest-timeout :: DummySubDir/OneTest.py/1/2' FAILED ******************** 
# | check:34'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            17: Script(shard): 
# | check:34'0     ~~~~~~~~~~~~~~~
...

@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 9, 2025

LLVM Buildbot has detected a new failure on builder clang-s390x-linux-lnt running on systemz-1 while building clang at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/136/builds/3085

Here is the relevant piece of the build log for the reference
Step 7 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'libFuzzer-s390x-default-Linux :: fuzzer-timeout.test' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/./bin/clang    -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/lib/fuzzer  /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/TimeoutTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/./bin/clang -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/lib/fuzzer /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/TimeoutTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest
RUN: at line 2: /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/./bin/clang    -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta   --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/lib/fuzzer  /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/TimeoutEmptyTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutEmptyTest
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/./bin/clang -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/lib/fuzzer /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/TimeoutEmptyTest.cpp -o /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutEmptyTest
RUN: at line 3: not  /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 2>&1 | FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=TimeoutTest
+ not /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1
+ FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=TimeoutTest
RUN: at line 12: not  /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/hi.txt 2>&1 | FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=SingleInputTimeoutTest
+ not /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/hi.txt
+ FileCheck /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/compiler-rt/test/fuzzer/fuzzer-timeout.test --check-prefix=SingleInputTimeoutTest
RUN: at line 16: /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 -timeout_exitcode=0
+ /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1/runtimes/runtimes-bins/compiler-rt/test/fuzzer/S390XDefaultLinuxConfig/Output/fuzzer-timeout.test.tmp-TimeoutTest -timeout=1 -timeout_exitcode=0
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2564076980
INFO: Loaded 1 modules   (13 inline 8-bit counters): 13 [0x2aa008d0e80, 0x2aa008d0e8d), 
INFO: Loaded 1 PC tables (13 PCs): 13 [0x2aa008d0e90,0x2aa008d0f60), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2	INITED cov: 2 ft: 2 corp: 1/1b exec/s: 0 rss: 31Mb
#1891	NEW    cov: 3 ft: 3 corp: 2/22b lim: 21 exec/s: 0 rss: 32Mb L: 21/21 MS: 4 CrossOver-InsertByte-ChangeBinInt-InsertRepeatedBytes-
#1892	REDUCE cov: 3 ft: 3 corp: 2/19b lim: 21 exec/s: 0 rss: 32Mb L: 18/18 MS: 1 EraseBytes-
#1924	REDUCE cov: 3 ft: 3 corp: 2/14b lim: 21 exec/s: 0 rss: 32Mb L: 13/13 MS: 2 ChangeBit-EraseBytes-
#1951	REDUCE cov: 3 ft: 3 corp: 2/12b lim: 21 exec/s: 0 rss: 32Mb L: 11/11 MS: 2 ChangeBinInt-EraseBytes-
#2074	REDUCE cov: 3 ft: 3 corp: 2/5b lim: 21 exec/s: 0 rss: 32Mb L: 4/4 MS: 3 ChangeBit-CopyPart-CrossOver-
#2126	REDUCE cov: 3 ft: 3 corp: 2/4b lim: 21 exec/s: 0 rss: 32Mb L: 3/3 MS: 2 ChangeByte-EraseBytes-
#2199	NEW    cov: 4 ft: 4 corp: 3/5b lim: 21 exec/s: 0 rss: 32Mb L: 1/3 MS: 3 CopyPart-ShuffleBytes-CrossOver-
#2362	REDUCE cov: 4 ft: 4 corp: 3/4b lim: 21 exec/s: 0 rss: 32Mb L: 2/2 MS: 3 EraseBytes-ChangeBinInt-ShuffleBytes-
#7153	REDUCE cov: 5 ft: 5 corp: 4/7b lim: 68 exec/s: 0 rss: 32Mb L: 3/3 MS: 1 InsertByte-
#7235	NEW    cov: 6 ft: 6 corp: 5/9b lim: 68 exec/s: 0 rss: 32Mb L: 2/3 MS: 2 InsertByte-EraseBytes-
ALARM: working on the last Unit for 1 seconds
       and the timeout value is 1 (use -timeout=N to change)
MS: 4 CrossOver-ChangeBinInt-EraseBytes-InsertByte-; base unit: 94dd9e08c129c785f7f256e82fbe0a30e6d1ae40
0x48,0x69,0x21,0x5,0xa,
Hi!\005\012
artifact_prefix='./'; Test unit written to ./timeout-9d0ed52845d8d0e027bf8fcefb26c5f60903b283
Base64: SGkhBQo=
==689329== ERROR: libFuzzer: timeout after 1 seconds
AddressSanitizer:DEADLYSIGNAL
=================================================================
AddressSanitizer:DEADLYSIGNAL
=================================================================
AddressSanitizer: CHECK failed: asan_report.cpp:199 "((current_error_.kind)) == ((kErrorKindInvalid))" (0x1, 0x0) (tid=689329)
    <empty stack>

MS: 4 CrossOver-ChangeBinInt-EraseBytes-InsertByte-; base unit: 94dd9e08c129c785f7f256e82fbe0a30e6d1ae40
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

7 participants