Skip to content

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Sep 17, 2025

We have inconsistent handling of null returns on target MC constructors. Most tools report an error, but some assert. It's currently not possible to test this with any in-tree targets. Make this a clean error so in the future targets can fail the construction.

Copy link
Contributor Author

arsenm commented Sep 17, 2025

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

@arsenm arsenm added the llvm-tools All llvm tools that do not have corresponding tag label Sep 17, 2025 — with Graphite App
@arsenm arsenm marked this pull request as ready for review September 17, 2025 01:48
@llvmbot llvmbot added the llvm:mc Machine (object) code label Sep 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2025

@llvm/pr-subscribers-llvm-mc

Author: Matt Arsenault (arsenm)

Changes

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

1 Files Affected:

  • (modified) llvm/tools/llvm-mc/llvm-mc.cpp (+4-1)
diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp
index 136cd69526a3c..224fd80f6a6d3 100644
--- a/llvm/tools/llvm-mc/llvm-mc.cpp
+++ b/llvm/tools/llvm-mc/llvm-mc.cpp
@@ -469,7 +469,10 @@ int main(int argc, char **argv) {
 
   std::unique_ptr<MCSubtargetInfo> STI(
       TheTarget->createMCSubtargetInfo(TheTriple, MCPU, FeaturesStr));
-  assert(STI && "Unable to create subtarget info!");
+  if (!STI) {
+    WithColor::error(errs(), ProgName) << "unable to create subtarget info\n";
+    return 1;
+  }
 
   // FIXME: This is not pretty. MCContext has a ptr to MCObjectFileInfo and
   // MCObjectFileInfo needs a MCContext reference in order to initialize itself.

@MaskRay
Copy link
Member

MaskRay commented Sep 17, 2025

Can you add a test to llvm/test/tools/llvm-mc?

% llvm-mc -triple=xxx
llvm-mc: error: unable to get target for 'xxx', see --version and --triple.

@arsenm
Copy link
Contributor Author

arsenm commented Sep 17, 2025

Can you add a test to llvm/test/tools/llvm-mc?

Short answer is no

% llvm-mc -triple=xxx
llvm-mc: error: unable to get target for 'xxx', see --version and --triple.

This is a different error for the top level target machine. There is no current in tree example where the subtarget constructor will return nullptr (but it probably should be a failable method that returns with error messages). Currently this would only trigger on partially implemented targets

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.

Should edit the description that testing is not feasible.

@arsenm arsenm merged commit ac1012d into main Sep 17, 2025
14 checks passed
@arsenm arsenm deleted the users/arsenm/llvm-mc/error-mcsubtargetinfo-construct-failure branch September 17, 2025 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:mc Machine (object) code llvm-tools All llvm tools that do not have corresponding tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants