[llvm] 2c67f2f - [llvm] Add format check for MCSubtargetFeatures (#180943)

[via llvm-commits]

Author: Georgiy Samoylov
Date: 2026-04-16T17:17:41+03:00
New Revision: 2c67f2fdfc41afd9acfe28e3cb42850a97f1db54

URL: https://github.com/llvm/llvm-project/commit/2c67f2fdfc41afd9acfe28e3cb42850a97f1db54
DIFF: https://github.com/llvm/llvm-project/commit/2c67f2fdfc41afd9acfe28e3cb42850a97f1db54.diff

LOG: [llvm] Add format check for MCSubtargetFeatures (#180943)

`SubtargetFeatures` class has next constraints:
https://github.com/llvm/llvm-project/blob/c9d065abc15846deb95a23fb0b3e1855d3d26314/llvm/include/llvm/TargetParser/SubtargetFeature.h#L167-L174

At this moment feature string isn't checked for fitting in such format.
This leads to assertion failure, for example in lldb:
https://github.com/llvm/llvm-project/pull/180901, when features from
user's input don't meet the requirements.

With implementing additional format check we can avoid such problems.

Added: 
    

Modified: 
    llvm/include/llvm/MC/TargetRegistry.h
    llvm/lib/MC/TargetRegistry.cpp
    llvm/unittests/MC/TargetRegistry.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/TargetRegistry.h b/llvm/include/llvm/MC/TargetRegistry.h
index 5594ede0aa5c1..6b45274f9dc95 100644
--- a/llvm/include/llvm/MC/TargetRegistry.h
+++ b/llvm/include/llvm/MC/TargetRegistry.h
@@ -466,6 +466,8 @@ class Target {
                                          StringRef Features) const {
     if (!MCSubtargetInfoCtorFn)
       return nullptr;
+    if (!isValidFeatureListFormat(Features))
+      return nullptr;
     return MCSubtargetInfoCtorFn(TheTriple, CPU, Features);
   }
 
@@ -652,6 +654,20 @@ class Target {
     return nullptr;
   }
 
+  /// isValidFeatureListFormat - check that FeatureString
+  /// has the format:
+  ///   "+attr1,+attr2,-attr3,...,+attrN"
+  /// A comma separates each feature from the next (all lowercase).
+  /// Each of the remaining features is prefixed with '+' or '-' indicating
+  /// whether that feature should be enabled or disabled contrary to the cpu
+  /// specification.
+  /// The string must match exactly that format otherwise
+  /// MCSubtargetInfo::ApplyFeatureFlag will fail.
+  /// For example feature string "+a,+m,c" is accepted, and results in feature
+  /// list {"+a", "+m", "c"}. Later in ApplyFeatureFlag, it asserts
+  /// that all features must start with '+' or '-' and assert is failed.
+  static bool isValidFeatureListFormat(StringRef FeaturesString);
+
   /// @}
 };
 

diff  --git a/llvm/lib/MC/TargetRegistry.cpp b/llvm/lib/MC/TargetRegistry.cpp
index db743ef9bdcb6..948fb5034f726 100644
--- a/llvm/lib/MC/TargetRegistry.cpp
+++ b/llvm/lib/MC/TargetRegistry.cpp
@@ -16,6 +16,7 @@
 #include "llvm/MC/MCLFI.h"
 #include "llvm/MC/MCObjectStreamer.h"
 #include "llvm/MC/MCObjectWriter.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <vector>
@@ -24,6 +25,14 @@ using namespace llvm;
 // Clients are responsible for avoid race conditions in registration.
 static Target *FirstTarget = nullptr;
 
+bool Target::isValidFeatureListFormat(StringRef Features) {
+  if (Features.empty())
+    return true;
+
+  static const llvm::Regex pattern("^([+-][^,]+)(,[+-][^,]+)*,?$");
+  return pattern.match(Features);
+}
+
 MCStreamer *Target::createMCObjectStreamer(
     const Triple &T, MCContext &Ctx, std::unique_ptr<MCAsmBackend> TAB,
     std::unique_ptr<MCObjectWriter> OW, std::unique_ptr<MCCodeEmitter> Emitter,

diff  --git a/llvm/unittests/MC/TargetRegistry.cpp b/llvm/unittests/MC/TargetRegistry.cpp
index cc464b7681ce1..f06072d4d9647 100644
--- a/llvm/unittests/MC/TargetRegistry.cpp
+++ b/llvm/unittests/MC/TargetRegistry.cpp
@@ -42,4 +42,51 @@ TEST(TargetRegistry, TargetHasArchType) {
   ASSERT_NE(Count, 0);
 }
 
+TEST(TargetRegistry, IsValidFeatureListFormat) {
+  // Valid strings
+
+  // Empty string is a valid feature string
+  EXPECT_TRUE(Target::isValidFeatureListFormat(""));
+
+  EXPECT_TRUE(Target::isValidFeatureListFormat("+some_feature"));
+  EXPECT_TRUE(Target::isValidFeatureListFormat("-some_feature"));
+  EXPECT_TRUE(
+      Target::isValidFeatureListFormat("+feature1,-feature2,+feature3"));
+  EXPECT_TRUE(Target::isValidFeatureListFormat("+123"));
+
+  // Strings with single trailing comma are also valid
+  EXPECT_TRUE(Target::isValidFeatureListFormat("-feature,"));
+  EXPECT_TRUE(
+      Target::isValidFeatureListFormat("-feature1,+feature2,+feature3,"));
+
+  // Invalid strings
+
+  // Feature don't start with '+' or '-'
+  EXPECT_FALSE(Target::isValidFeatureListFormat("invalid_string"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat("+good,bad"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat("bad,+good"));
+
+  // String has spaces
+  EXPECT_FALSE(Target::isValidFeatureListFormat(" "));
+  EXPECT_FALSE(Target::isValidFeatureListFormat(", "));
+  EXPECT_FALSE(Target::isValidFeatureListFormat(" avx"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat("+avx, -sse"));
+
+  // Redundant commas
+  EXPECT_FALSE(Target::isValidFeatureListFormat("+feature1,,+feature2"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat(",+feature"));
+  EXPECT_FALSE(
+      Target::isValidFeatureListFormat("+feature1,,,+feature2,,+feature3"));
+
+  // Feature consists only of '+' or '-'
+  EXPECT_FALSE(Target::isValidFeatureListFormat("+"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat("-"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat("+avx,-"));
+
+  // Only commas
+  EXPECT_FALSE(Target::isValidFeatureListFormat(","));
+  EXPECT_FALSE(Target::isValidFeatureListFormat(",,"));
+  EXPECT_FALSE(Target::isValidFeatureListFormat(",,,"));
+}
+
 } // end namespace