JDevlieghere created this revision.
JDevlieghere added reviewers: davide, sgraenitz, friss.
Herald added subscribers: teemperor, jfb.
Herald added a project: LLDB.
The original intention was to change the watchpoint Python test to a lit test.
While doing so, I found a situation where stop-command-source-on-error was
being ignored. Since the new test also tests the fix for the
CommandObjectCommands, I figured I'd keep them in a single patch.
- I converted the test to lit because it is flaky on GreenDragon.
- The problem with the command object is that we didn't differentiate between
the boolean value not being set, and the value being set to either true or
false. If the value isn't set, we'd calculate it in the command interpreter
based on the global options. With the old approach, we'd always set the value
to the default of the option, which is not what the user wanted.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D61406
Files:
lldb/lit/Watchpoint/Inputs/main.cpp
lldb/lit/Watchpoint/SetErrorCases.test
lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchpointSetErrorCases.py
lldb/source/Commands/CommandObjectCommands.cpp
Index: lldb/source/Commands/CommandObjectCommands.cpp
===================================================================
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -309,8 +309,13 @@
m_options.m_stop_on_continue.OptionWasSet()) {
// Use user set settings
CommandInterpreterRunOptions options;
- options.SetStopOnContinue(m_options.m_stop_on_continue.GetCurrentValue());
- options.SetStopOnError(m_options.m_stop_on_error.GetCurrentValue());
+
+ if (m_options.m_stop_on_continue.OptionWasSet())
+ options.SetStopOnContinue(
+ m_options.m_stop_on_continue.GetCurrentValue());
+
+ if (m_options.m_stop_on_error.OptionWasSet())
+ options.SetStopOnError(m_options.m_stop_on_error.GetCurrentValue());
// Individual silent setting is override for global command echo settings.
if (m_options.m_silent_run.GetCurrentValue()) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchpointSetErrorCases.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchpointSetErrorCases.py
+++ /dev/null
@@ -1,74 +0,0 @@
-"""
-Test error cases for the 'watchpoint set' command to make sure it errors out when necessary.
-"""
-
-from __future__ import print_function
-
-
-import os
-import time
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
-
-
-class WatchpointSetErrorTestCase(TestBase):
-
- mydir = TestBase.compute_mydir(__file__)
-
- def setUp(self):
- # Call super's setUp().
- TestBase.setUp(self)
- # Our simple source filename.
- self.source = 'main.cpp'
- # Find the line number to break inside main().
- self.line = line_number(
- self.source, '// Set break point at this line.')
- # Build dictionary to have unique executable names for each test
- # method.
-
- def test_error_cases_with_watchpoint_set(self):
- """Test error cases with the 'watchpoint set' command."""
- self.build()
- self.setTearDownCleanup()
-
- exe = self.getBuildArtifact("a.out")
- self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
- # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
- lldbutil.run_break_set_by_file_and_line(
- self, None, self.line, num_expected_locations=1)
-
- # Run the program.
- self.runCmd("run", RUN_SUCCEEDED)
-
- # We should be stopped again due to the breakpoint.
- # The stop reason of the thread should be breakpoint.
- self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
- substrs=['stopped',
- 'stop reason = breakpoint'])
-
- # Try some error conditions:
-
- # 'watchpoint set' is now a multiword command.
- self.expect("watchpoint set",
- substrs=['The following subcommands are supported:',
- 'expression',
- 'variable'])
- self.runCmd("watchpoint set variable -w read_write", check=False)
-
- # 'watchpoint set expression' with '-w' or '-s' specified now needs
- # an option terminator and a raw expression after that.
- self.expect("watchpoint set expression -w write --", error=True,
- startstr='error: ')
-
- # It's an error if the expression did not evaluate to an address.
- self.expect(
- "watchpoint set expression MyAggregateDataType",
- error=True,
- startstr='error: expression did not evaluate to an address')
-
- # Wrong size parameter is an error.
- self.expect("watchpoint set variable -s -128", error=True,
- substrs=['invalid enumeration value'])
Index: lldb/lit/Watchpoint/SetErrorCases.test
===================================================================
--- /dev/null
+++ lldb/lit/Watchpoint/SetErrorCases.test
@@ -0,0 +1,28 @@
+# RUN: %clangxx %p/Inputs/main.cpp -g -o %t.out
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s %t.out 2>&1 | FileCheck %s
+
+settings show interpreter.stop-command-source-on-error
+# CHECK: interpreter.stop-command-source-on-error (boolean) = false
+
+b main.cpp:110
+run
+# CHECK: stopped
+# CHECK-NEXT: stop reason = breakpoint
+
+watchpoint set
+# CHECK: Commands for setting a watchpoint.
+# CHECK: The following subcommands are supported:
+# CHECK: Set a watchpoint on an address by supplying an expression.
+# CHECK: Set a watchpoint on a variable.
+
+watchpoint set variable -w read_write
+# CHECK: error: required argument missing
+
+watchpoint set expression -w write --
+# CHECK: error: expression evaluation of address to watch failed
+
+watchpoint set expression MyAggregateDataType
+# CHECK: error: expression did not evaluate to an address
+
+watchpoint set variable -s -128
+# CHECK: error: invalid enumeration value
Index: lldb/lit/Watchpoint/Inputs/main.cpp
===================================================================
--- /dev/null
+++ lldb/lit/Watchpoint/Inputs/main.cpp
@@ -0,0 +1,120 @@
+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <chrono>
+#include <condition_variable>
+#include <cstdio>
+#include <random>
+#include <thread>
+
+std::default_random_engine g_random_engine{std::random_device{}()};
+std::uniform_int_distribution<> g_distribution{0, 3000000};
+std::condition_variable g_condition_variable;
+std::mutex g_mutex;
+int g_count;
+
+char *g_char_ptr = nullptr;
+
+void
+barrier_wait()
+{
+ std::unique_lock<std::mutex> lock{g_mutex};
+ if (--g_count > 0)
+ g_condition_variable.wait(lock);
+ else
+ g_condition_variable.notify_all();
+}
+
+void
+do_bad_thing_with_location(unsigned index, char *char_ptr, char new_val)
+{
+ unsigned what = new_val;
+ printf("new value written to array(%p) and index(%u) = %u\n", char_ptr, index, what);
+ char_ptr[index] = new_val;
+}
+
+uint32_t
+access_pool (bool flag = false)
+{
+ static std::mutex g_access_mutex;
+ static unsigned idx = 0; // Well-behaving thread only writes into indexs from 0..6.
+ if (!flag)
+ g_access_mutex.lock();
+
+ // idx valid range is [0, 6].
+ if (idx > 6)
+ idx = 0;
+
+ if (flag)
+ {
+ // Write into a forbidden area.
+ do_bad_thing_with_location(7, g_char_ptr, 99);
+ }
+
+ unsigned index = idx++;
+
+ if (!flag)
+ g_access_mutex.unlock();
+ return g_char_ptr[index];
+}
+
+void
+thread_func (uint32_t thread_index)
+{
+ printf ("%s (thread index = %u) startng...\n", __FUNCTION__, thread_index);
+
+ barrier_wait();
+
+ uint32_t count = 0;
+ uint32_t val;
+ while (count++ < 15)
+ {
+ // random micro second sleep from zero to 3 seconds
+ int usec = g_distribution(g_random_engine);
+ printf ("%s (thread = %u) doing a usleep (%d)...\n", __FUNCTION__, thread_index, usec);
+ std::this_thread::sleep_for(std::chrono::microseconds{usec});
+
+ if (count < 7)
+ val = access_pool ();
+ else
+ val = access_pool (true);
+
+ printf ("%s (thread = %u) after usleep access_pool returns %d (count=%d)...\n", __FUNCTION__, thread_index, val, count);
+ }
+ printf ("%s (thread index = %u) exiting...\n", __FUNCTION__, thread_index);
+}
+
+
+int main (int argc, char const *argv[])
+{
+ g_count = 4;
+ std::thread threads[3];
+
+ g_char_ptr = new char[10]{};
+
+ // Create 3 threads
+ for (auto &thread : threads)
+ thread = std::thread{thread_func, std::distance(threads, &thread)};
+
+ struct {
+ int a;
+ int b;
+ int c;
+ } MyAggregateDataType;
+
+ printf ("Before turning all three threads loose...\n"); // Set break point at this line.
+ barrier_wait();
+
+ // Join all of our threads
+ for (auto &thread : threads)
+ thread.join();
+
+ delete[] g_char_ptr;
+
+ return 0;
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
