Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 17, 2025

We do not have consistent or good error handling of this situation.
Some tools check for errors, some just assert. The backend has no
proper way of reporting an invalid subtarget specification. MCSubtargetInfo
currently does unreasonable things like spam warnings to errs, and then
silently proceed in an invalid state. I have a patch which starts returning
null on some invalid subtargets, but all the tools need to start erroring
cleanly first. I don't think there is a reliable way to test this today. It
would have to be an incomplete backend. Ideally we would thread through
some kind of error context from the target to report the reason it's
an invalid subtarget.

We do not have consistent or good error handling of this situation.
Some tools check for errors, some just assert. The backend has no
proper way of reporting an invalid subtarget specification. MCSubtargetInfo
currently does unreasonable things like spam warnings to errs, and then
silently proceed in an invalid state. I have a patch which starts returning
null on some invalid subtargets, but all the tools need to start erroring
cleanly first. I don't think there is a reliable way to test this today. It
would have to be an incomplete backend. Ideally we would thread through
some kind of error context from the target to report the reason it's
an invalid subtarget.
@arsenm arsenm added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' llvm:mc Machine (object) code labels Sep 17, 2025 — with Graphite App
Copy link
Contributor Author

arsenm commented Sep 17, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2025

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-llvm-mc

@llvm/pr-subscribers-clang-driver

Author: Matt Arsenault (arsenm)

Changes

We do not have consistent or good error handling of this situation.
Some tools check for errors, some just assert. The backend has no
proper way of reporting an invalid subtarget specification. MCSubtargetInfo
currently does unreasonable things like spam warnings to errs, and then
silently proceed in an invalid state. I have a patch which starts returning
null on some invalid subtargets, but all the tools need to start erroring
cleanly first. I don't think there is a reliable way to test this today. It
would have to be an incomplete backend. Ideally we would thread through
some kind of error context from the target to report the reason it's
an invalid subtarget.


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

2 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticFrontendKinds.td (+2)
  • (modified) clang/tools/driver/cc1as_main.cpp (+4-1)
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 2fd2ae434d7c5..254b13be005bb 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -115,6 +115,8 @@ def err_fe_unable_to_load_plugin : Error<
     "unable to load plugin '%0': '%1'">;
 def err_fe_unable_to_create_target : Error<
     "unable to create target: '%0'">;
+def err_fe_unable_to_create_subtarget : Error<
+    "unable to create subtarget: '%0'%select{ with features '%2'|}1">;
 def err_fe_unable_to_interface_with_target : Error<
     "unable to interface with target machine">;
 def err_fe_unable_to_open_output : Error<
diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp
index 5c30de33c7b46..dc0f74957774a 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -480,7 +480,10 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
 
   std::unique_ptr<MCSubtargetInfo> STI(
       TheTarget->createMCSubtargetInfo(Opts.Triple, Opts.CPU, FS));
-  assert(STI && "Unable to create subtarget info!");
+  if (!STI) {
+    return Diags.Report(diag::err_fe_unable_to_create_subtarget)
+           << Opts.CPU << FS.empty() << FS;
+  }
 
   MCContext Ctx(Triple(Opts.Triple), MAI.get(), MRI.get(), STI.get(), &SrcMgr,
                 &MCOptions);

@arsenm arsenm marked this pull request as ready for review September 17, 2025 01:28
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 17, 2025
Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

Can you add a clang/test/Misc/cc1as-* test?

@arsenm
Copy link
Contributor Author

arsenm commented Sep 17, 2025

Can you add a clang/test/Misc/cc1as-* test?

This is the same as llvm-mc, can't produce a construction failure with any of the targets as is

Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

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

if manual testing locally shows that the error message is correct then lgtm

@arsenm arsenm merged commit d46715a into main Sep 17, 2025
16 checks passed
@arsenm arsenm deleted the users/arsenm/clang/error-mcsubtarget-failure branch September 17, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants