[lldb] Allow limiting the number of error diagnostics when parsing an expression

[teemperor]

Summary

While debugging another bug I found out that we currently don't set any limit for the
number of diagnostics Clang emits. If a user does something that generates a lot of
errors (like including some long header file from within the expression function), then
we currently spam the LLDB output with potentially thousands of Clang error diagnostics.

Clang sets a default limit of 20 errors, but given that LLDB is often used interactively
for small expressions I would say a limit of 5 is enough. The limit is implemented as a
setting, so if a user cares about seeing having a million errors printed to their terminal
then they can just increase the settings value.

[mib]

LGTM! Have you considered also adding an option to the expression command object ? May be some people would like to change the error limit for a specific expression...

[mib]

lldb/include/lldb/Target/Target.h

176

Nit: Could you rename this to GetExpressionErrorLimit to stay consistent with the previous method ?

[teemperor]

In D88889#2314156, @mib wrote:

LGTM! Have you considered also adding an option to the expression command object ? May be some people would like to change the error limit for a specific expression...

I think that would also work. Not sure if we really have a pattern for whether an option should be a setting or not, but I usually add really obscure options that 99.99% of users never change to the settings. Mostly because this way they don't clutter the expr help page (which is already huge) and we don't have to add all the respective code permanently to SWIG/SBExpressionOptions/ExpressionOption.

[shafik]

LGTM

lldb/include/lldb/Target/Target.h

public:
   void SetStandardErrorPath(const char *path) = delete;
 
   bool GetBreakpointsConsultPlatformAvoidList();
 
   lldb::LanguageType GetLanguage() const;
 
   llvm::StringRef GetExpressionPrefixContents();
 
+  uint64_t GetExprErrorLimit() const;
+
   bool GetUseHexImmediates() const;
 
   bool GetUseFastStepping() const;
 
   bool GetDisplayExpressionsInCrashlogs() const;
 
   LoadScriptFromSymFile GetLoadScriptFromSymbolFile() const;
 

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

if (log && log->GetVerbose()) {
       LLDB_LOGV(log, "ABI: '{0}'", opts.ABI);
       LLDB_LOGV(log, "LinkerVersion: '{0}'", opts.LinkerVersion);
       StringList::LogDump(log, opts.FeaturesAsWritten, "FeaturesAsWritten");
       StringList::LogDump(log, opts.Features, "Features");
     }
 
   // 4. Create and install the target on the compiler.
   m_compiler->createDiagnostics();
+  // Limit the number of error diagnostics we emit.
+  // A value of 0 means no limit for both LLDB and Clang.
+  m_compiler->getDiagnostics().setErrorLimit(target_sp->GetExprErrorLimit());
+
   auto target_info = TargetInfo::CreateTargetInfo(
       m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
     LLDB_LOGF(log, "Using SIMD alignment: %d",
               target_info->getSimdDefaultAlign());
     LLDB_LOGF(log, "Target datalayout string: '%s'",
               target_info->getDataLayout().getStringRepresentation().c_str());
     LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());

lldb/source/Target/Target.cpp

if (file) {
     if (data_sp)
       return llvm::StringRef(
           reinterpret_cast<const char *>(data_sp->GetBytes()),
           data_sp->GetByteSize());
   }
   return "";
 }
 
+uint64_t TargetProperties::GetExprErrorLimit() const {
+  const uint32_t idx = ePropertyExprErrorLimit;
+  return m_collection_sp->GetPropertyAtIndexAsUInt64(
+      nullptr, idx, g_target_properties[idx].default_uint_value);
+}
+
 bool TargetProperties::GetBreakpointsConsultPlatformAvoidList() {
   const uint32_t idx = ePropertyBreakpointUseAvoidList;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_target_properties[idx].default_uint_value != 0);
 }
 
 bool TargetProperties::GetUseHexImmediates() const {
   const uint32_t idx = ePropertyUseHexImmediates;

lldb/source/Target/TargetProperties.td

def MoveToNearestCode: Property<"move-to-nearest-code", "Boolean">,
     DefaultTrue,
     Desc<"Move breakpoints to nearest code.">;
   def Language: Property<"language", "Language">,
     DefaultEnumValue<"eLanguageTypeUnknown">,
     Desc<"The language to use when interpreting expressions entered in commands.">;
   def ExprPrefix: Property<"expr-prefix", "FileSpec">,
     DefaultStringValue<"">,
     Desc<"Path to a file containing expressions to be prepended to all expressions.">;
+  def ExprErrorLimit: Property<"expr-error-limit", "UInt64">,
+    DefaultUnsignedValue<5>,
+    Desc<"The maximum amount of errors to emit while parsing an expression. "
+         "A value of 0 means to always continue parsing if possible.">;
   def PreferDynamic: Property<"prefer-dynamic-value", "Enum">,
     DefaultEnumValue<"eDynamicDontRunTarget">,
     EnumValues<"OptionEnumValues(g_dynamic_value_types)">,
     Desc<"Should printed values be shown as their dynamic value.">;
   def EnableSynthetic: Property<"enable-synthetic-value", "Boolean">,
     DefaultTrue,
     Desc<"Should synthetic values be used by default whenever available.">;
   def SkipPrologue: Property<"skip-prologue", "Boolean">,

lldb/test/API/commands/expression/error-limit/Makefile

+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/test/API/commands/expression/error-limit/TestExprErrorLimit.py

+"""
+Tests target.expr-error-limit.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test(self):
+        # FIXME: The only reason this test needs to create a real target is because
+        # the settings of the dummy target can't be changed with `settings set`.
+        self.build()
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        # Our test expression that is just several lines of malformed
+        # integer literals (with a 'yerror' integer suffix). Every error
+        # has its own unique string (1, 2, 3, 4) and is on its own line
+        # that we can later find it when Clang prints the respective source
+        # code for each error to the error output.
+        # For example, in the error output below we would look for the
+        # unique `1yerror` string:
+        #     error: <expr>:1:2: invalid suffix 'yerror' on integer constant
+        #     1yerror
+        #     ^
+        expr = "1yerror;\n2yerror;\n3yerror;\n4yerror;"
+
+        options = lldb.SBExpressionOptions()
+        options.SetAutoApplyFixIts(False)
+
+        # Evaluate the expression and check that only the first 2 errors are
+        # emitted.
+        self.runCmd("settings set target.expr-error-limit 2")
+        eval_result = target.EvaluateExpression(expr, options)
+        self.assertIn("1yerror", str(eval_result.GetError()))
+        self.assertIn("2yerror", str(eval_result.GetError()))
+        self.assertNotIn("3yerror", str(eval_result.GetError()))
+        self.assertNotIn("4yerror", str(eval_result.GetError()))
+
+        # Change to a 3 errors and check again which errors are emitted.
+        self.runCmd("settings set target.expr-error-limit 3")
+        eval_result = target.EvaluateExpression(expr, options)
+        self.assertIn("1yerror", str(eval_result.GetError()))
+        self.assertIn("2yerror", str(eval_result.GetError()))
+        self.assertIn("3yerror", str(eval_result.GetError()))
+        self.assertNotIn("4yerror", str(eval_result.GetError()))
+
+        # Disable the error limit and make sure all errors are emitted.
+        self.runCmd("settings set target.expr-error-limit 0")
+        eval_result = target.EvaluateExpression(expr, options)
+        self.assertIn("1yerror", str(eval_result.GetError()))
+        self.assertIn("2yerror", str(eval_result.GetError()))
+        self.assertIn("3yerror", str(eval_result.GetError()))
+        self.assertIn("4yerror", str(eval_result.GetError()))

lldb/test/API/commands/expression/error-limit/main.cpp

+int main() {
+  return 0; // break here
+}