[lldb] [llvm] [llvm][lldb] Add check for incorrect target features (PR #180901)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-mc

Author: Georgiy Samoylov (sga-sc)

<details>
<summary>Changes</summary>

Assertion failure was discovered during lldb usage after incorrect platform feature input:
```
(lldb) disassemble -Y rv64gc
'rv64gc' is not a recognized processor for this target (ignoring processor)
lldb: /home/llvm-project/llvm/lib/MC/MCSubtargetInfo.cpp:62: void ApplyFeatureFlag(FeatureBitset &, StringRef, ArrayRef<SubtargetFeatureKV>): Assertion `SubtargetFeatures::hasFlag(Feature) && "Feature flags should start with '+' or '-'"' failed.
LLDB diagnostics will be written to /tmp/diagnostics-48a5c1
```

This patch fixes this error with additional argument check for subtarget features

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


3 Files Affected:

- (modified) lldb/source/Commands/CommandObjectDisassemble.cpp (+9-9) 
- (added) lldb/test/API/commands/disassemble/basic/TestDisassembleInvalidTargetFeatures.py (+36) 
- (modified) llvm/include/llvm/MC/TargetRegistry.h (+28-1) 


``````````diff
diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp
index c0553d2c6c8b2..faed2cafc8237 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -486,15 +486,15 @@ void CommandObjectDisassemble::DoExecute(Args &command,
       m_options.arch, flavor_string, cpu_string, features_string, plugin_name);
 
   if (!disassembler) {
-    if (plugin_name) {
-      result.AppendErrorWithFormat(
-          "Unable to find Disassembler plug-in named '%s' that supports the "
-          "'%s' architecture.\n",
-          plugin_name, m_options.arch.GetArchitectureName());
-    } else
-      result.AppendErrorWithFormat(
-          "Unable to find Disassembler plug-in for the '%s' architecture.\n",
-          m_options.arch.GetArchitectureName());
+    result.AppendErrorWithFormat(
+        "Unable to find Disassembler plug-in with such parameters: name: '%s', "
+        "flavor: '%s', cpu: '%s', features: '%s' for the "
+        "'%s' architecture.\n",
+        plugin_name ? plugin_name : "None",
+        flavor_string ? flavor_string : "None",
+        cpu_string ? cpu_string : "None",
+        features_string ? features_string : "None",
+        m_options.arch.GetArchitectureName());
     return;
   } else if (flavor_string != nullptr && !disassembler->FlavorValidForArchSpec(
                                              m_options.arch, flavor_string))
diff --git a/lldb/test/API/commands/disassemble/basic/TestDisassembleInvalidTargetFeatures.py b/lldb/test/API/commands/disassemble/basic/TestDisassembleInvalidTargetFeatures.py
new file mode 100644
index 0000000000000..2e8c3c8d267e6
--- /dev/null
+++ b/lldb/test/API/commands/disassemble/basic/TestDisassembleInvalidTargetFeatures.py
@@ -0,0 +1,36 @@
+"""
+Test for lldb disassemble command with -Y option and invalid parameters.
+This test verifies that disassemble -Y command properly reports error messages
+when invoked with incorrect options.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import TestBase
+from lldbsuite.test import lldbutil
+
+
+class TestDisassembleInvalidTargetFeatures(TestBase):
+    """Test disassemble -Y option error handling."""
+
+    def run_invalid_disasm_cmd(self, option, expected_error):
+        cmd = f"disassemble -Y {option}"
+        self.runCmd(cmd, check=False)
+        output = self.res.GetOutput()
+        error = self.res.GetError()
+        self.assertFalse(self.res.Succeeded(), f"{cmd} should fail")
+        self.assertTrue(len(error) > 0, f"Error for '{cmd}' should not be empty")
+        self.assertIn(expected_error, error)
+
+    def test_disassemble_Y_invalid_options(self):
+        self.build()
+        _, _, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "main", lldb.SBFileSpec("main.cpp")
+        )
+
+        self.run_invalid_disasm_cmd("", "last option requires an argument")
+        self.run_invalid_disasm_cmd("invalid", "Unable to find Disassembler plug-in with such parameters:")
+        self.run_invalid_disasm_cmd("+invalid_1,-invalid_2,+invalid3", "Failed to disassemble memory at")
+        self.run_invalid_disasm_cmd("-Z", "Failed to disassemble memory at")
+        self.run_invalid_disasm_cmd("+++", "Failed to disassemble memory at")
+        self.run_invalid_disasm_cmd("----", "Failed to disassemble memory at")
diff --git a/llvm/include/llvm/MC/TargetRegistry.h b/llvm/include/llvm/MC/TargetRegistry.h
index 4451dfa72a5f4..6e2f194212875 100644
--- a/llvm/include/llvm/MC/TargetRegistry.h
+++ b/llvm/include/llvm/MC/TargetRegistry.h
@@ -351,6 +351,31 @@ class Target {
   /// InstrumentManager, if registered (default = nullptr).
   InstrumentManagerCtorTy InstrumentManagerCtorFn = nullptr;
 
+  bool isSignedFeatureList(StringRef FeaturesString) const {
+    if (FeaturesString.empty())
+      return true;
+
+    llvm::SmallVector<llvm::StringRef, 8> Features;
+    FeaturesString.split(Features, ',', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+
+    if (Features.empty())
+      return false;
+
+    for (llvm::StringRef Feature : Features) {
+      if (Feature.empty())
+        return false;
+
+      char C = Feature.front();
+      if (C != '+' && C != '-')
+        return false;
+
+      if (Feature.size() == 1)
+        return false;
+    }
+
+    return true;
+  }
+
 public:
   Target() = default;
 
@@ -452,7 +477,9 @@ class Target {
                                          StringRef Features) const {
     if (!MCSubtargetInfoCtorFn)
       return nullptr;
-    return MCSubtargetInfoCtorFn(TheTriple, CPU, Features);
+    if (!isSignedFeatureList(Features))
+      return nullptr;
+    return MCSubtargetInfoCtorFn(Triple(TheTriple), CPU, Features);
   }
 
   /// createTargetMachine - Create a target specific machine implementation

``````````

</details>


https://github.com/llvm/llvm-project/pull/180901