[Lldb-commits] [lldb] r283320 - cleanup RSCoordinate handling and factor out coordinate parser
Luke Drummond via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 5 07:34:52 PDT 2016
Author: ldrumm
Date: Wed Oct 5 09:34:52 2016
New Revision: 283320
URL: http://llvm.org/viewvc/llvm-project?rev=283320&view=rev
Log:
cleanup RSCoordinate handling and factor out coordinate parser
- This change updates the signature of
`RenderScriptRuntime::PlaceBreakpointOnKernel` to take a default
RSCoordinate pointer of nullptr. We use this as the predicate value for
the breakpoint coordinate rather than trying to fit a sentinel `-1` into
a signed version.
```
- void
- PlaceBreakpointOnKernel(Stream &strm, const char *name, const std::array<int, 3> coords, Error &error,
- lldb::TargetSP target);
```
```
+ bool
+ PlaceBreakpointOnKernel(lldb::TargetSP target, Stream &messages, const char *name,
+ const lldb_renderscript::RSCoordinate *coords = nullptr);
```
The above change makes the API for setting breakpoints on kernels
cleaner as it returns a failure value rather than modify a sentinel in
the caller. The optional arguments are now last and have a default
(falsey) value.
- RSCoordinate objects are now comparable with operator== and have
zero initializers which should make them easier to work on.
- Added a `FMT_COORD` macro for use in logging format strings which
should make format strings a little less verbose.
Modified:
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
Modified: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=283320&r1=283319&r2=283320&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp (original)
+++ lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp Wed Oct 5 09:34:52 2016
@@ -43,6 +43,8 @@ using namespace lldb;
using namespace lldb_private;
using namespace lldb_renderscript;
+#define FMT_COORD "(%" PRIu32 ", %" PRIu32 ", %" PRIu32 ")"
+
namespace {
// The empirical_type adds a basic level of validation to arbitrary data
@@ -429,6 +431,43 @@ bool GetArgs(ExecutionContext &context,
return false;
}
}
+
+bool ParseCoordinate(llvm::StringRef coord_s, RSCoordinate &coord) {
+ // takes an argument of the form 'num[,num][,num]'.
+ // Where 'coord_s' is a comma separated 1,2 or 3-dimensional coordinate
+ // with the whitespace trimmed.
+ // Missing coordinates are defaulted to zero.
+ // If parsing of any elements fails the contents of &coord are undefined
+ // and `false` is returned, `true` otherwise
+
+ RegularExpression regex;
+ RegularExpression::Match regex_match(3);
+
+ bool matched = false;
+ if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
+ regex.Execute(coord_s, ®ex_match))
+ matched = true;
+ else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
+ regex.Execute(coord_s, ®ex_match))
+ matched = true;
+ else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
+ regex.Execute(coord_s, ®ex_match))
+ matched = true;
+
+ if (!matched)
+ return false;
+
+ auto get_index = [&](int idx, uint32_t &i) -> bool {
+ std::string group;
+ errno = 0;
+ if (regex_match.GetMatchAtIndex(coord_s.str().c_str(), idx + 1, group))
+ return !llvm::StringRef(group).getAsInteger<uint32_t>(10, i);
+ return true;
+ };
+
+ return get_index(0, coord.x) && get_index(1, coord.y) &&
+ get_index(2, coord.z);
+}
} // anonymous namespace
// The ScriptDetails class collects data associated with a single script
@@ -3285,9 +3324,9 @@ bool RenderScriptRuntime::GetFrameVarAsU
// and false otherwise.
bool RenderScriptRuntime::GetKernelCoordinate(RSCoordinate &coord,
Thread *thread_ptr) {
- static const std::string s_runtimeExpandSuffix(".expand");
- static const std::array<const char *, 3> s_runtimeCoordVars{
- {"rsIndex", "p->current.y", "p->current.z"}};
+ static const char *const x_expr = "rsIndex";
+ static const char *const y_expr = "p->current.y";
+ static const char *const z_expr = "p->current.z";
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_LANGUAGE));
@@ -3311,48 +3350,38 @@ bool RenderScriptRuntime::GetKernelCoord
// Find the function name
const SymbolContext sym_ctx = frame_sp->GetSymbolContext(false);
- const char *func_name_cstr = sym_ctx.GetFunctionName().AsCString();
- if (!func_name_cstr)
+ const ConstString func_name = sym_ctx.GetFunctionName();
+ if (!func_name)
continue;
if (log)
log->Printf("%s - Inspecting function '%s'", __FUNCTION__,
- func_name_cstr);
+ func_name.GetCString());
// Check if function name has .expand suffix
- std::string func_name(func_name_cstr);
- const int length_difference =
- func_name.length() - s_runtimeExpandSuffix.length();
- if (length_difference <= 0)
- continue;
-
- const int32_t has_expand_suffix =
- func_name.compare(length_difference, s_runtimeExpandSuffix.length(),
- s_runtimeExpandSuffix);
-
- if (has_expand_suffix != 0)
+ if (!func_name.GetStringRef().endswith(".expand"))
continue;
if (log)
log->Printf("%s - Found .expand function '%s'", __FUNCTION__,
- func_name_cstr);
+ func_name.GetCString());
// Get values for variables in .expand frame that tell us the current kernel
// invocation
- bool found_coord_variables = true;
- assert(s_runtimeCoordVars.size() == coord.size());
-
- for (uint32_t i = 0; i < coord.size(); ++i) {
- uint64_t value = 0;
- if (!GetFrameVarAsUnsigned(frame_sp, s_runtimeCoordVars[i], value)) {
- found_coord_variables = false;
- break;
- }
- coord[i] = value;
- }
+ uint64_t x, y, z;
+ bool found = GetFrameVarAsUnsigned(frame_sp, x_expr, x) &&
+ GetFrameVarAsUnsigned(frame_sp, y_expr, y) &&
+ GetFrameVarAsUnsigned(frame_sp, z_expr, z);
- if (found_coord_variables)
+ if (found) {
+ // The RenderScript runtime uses uint32_t for these vars. If they're not
+ // within bounds, our frame parsing is garbage
+ assert(x <= UINT32_MAX && y <= UINT32_MAX && z <= UINT32_MAX);
+ coord.x = (uint32_t)x;
+ coord.y = (uint32_t)y;
+ coord.z = (uint32_t)z;
return true;
+ }
}
return false;
}
@@ -3377,13 +3406,11 @@ bool RenderScriptRuntime::KernelBreakpoi
"Error: null baton in conditional kernel breakpoint callback");
// Coordinate we want to stop on
- const uint32_t *target_coord = static_cast<const uint32_t *>(baton);
+ RSCoordinate target_coord = *static_cast<RSCoordinate *>(baton);
if (log)
- log->Printf("%s - Break ID %" PRIu64 ", (%" PRIu32 ", %" PRIu32 ", %" PRIu32
- ")",
- __FUNCTION__, break_id, target_coord[0], target_coord[1],
- target_coord[2]);
+ log->Printf("%s - Break ID %" PRIu64 ", " FMT_COORD, __FUNCTION__, break_id,
+ target_coord.x, target_coord.y, target_coord.z);
// Select current thread
ExecutionContext context(ctx->exe_ctx_ref);
@@ -3391,7 +3418,7 @@ bool RenderScriptRuntime::KernelBreakpoi
assert(thread_ptr && "Null thread pointer");
// Find current kernel invocation from .expand frame variables
- RSCoordinate current_coord{}; // Zero initialise array
+ RSCoordinate current_coord{};
if (!GetKernelCoordinate(current_coord, thread_ptr)) {
if (log)
log->Printf("%s - Error, couldn't select .expand stack frame",
@@ -3400,18 +3427,15 @@ bool RenderScriptRuntime::KernelBreakpoi
}
if (log)
- log->Printf("%s - (%" PRIu32 ",%" PRIu32 ",%" PRIu32 ")", __FUNCTION__,
- current_coord[0], current_coord[1], current_coord[2]);
+ log->Printf("%s - " FMT_COORD, __FUNCTION__, current_coord.x,
+ current_coord.y, current_coord.z);
// Check if the current kernel invocation coordinate matches our target
// coordinate
- if (current_coord[0] == target_coord[0] &&
- current_coord[1] == target_coord[1] &&
- current_coord[2] == target_coord[2]) {
- if (log)
- log->Printf("%s, BREAKING (%" PRIu32 ",%" PRIu32 ",%" PRIu32 ")",
- __FUNCTION__, current_coord[0], current_coord[1],
- current_coord[2]);
+ if (target_coord == current_coord) {
+ if (log)
+ log->Printf("%s, BREAKING " FMT_COORD, __FUNCTION__, current_coord.x,
+ current_coord.y, current_coord.z);
BreakpointSP breakpoint_sp =
context.GetTargetPtr()->GetBreakpointByID(break_id);
@@ -3426,50 +3450,52 @@ bool RenderScriptRuntime::KernelBreakpoi
return false;
}
+void RenderScriptRuntime::SetConditional(BreakpointSP bp, Stream &messages,
+ const RSCoordinate &coord) {
+ messages.Printf("Conditional kernel breakpoint on coordinate " FMT_COORD,
+ coord.x, coord.y, coord.z);
+ messages.EOL();
+
+ // Allocate memory for the baton, and copy over coordinate
+ RSCoordinate *baton = new RSCoordinate(coord);
+
+ // Create a callback that will be invoked every time the breakpoint is hit.
+ // The baton object passed to the handler is the target coordinate we want to
+ // break on.
+ bp->SetCallback(KernelBreakpointHit, baton, true);
+
+ // Store a shared pointer to the baton, so the memory will eventually be
+ // cleaned up after destruction
+ m_conditional_breaks[bp->GetID()] = std::unique_ptr<RSCoordinate>(baton);
+}
+
// Tries to set a breakpoint on the start of a kernel, resolved using the kernel
// name.
// Argument 'coords', represents a three dimensional coordinate which can be
// used to specify
// a single kernel instance to break on. If this is set then we add a callback
// to the breakpoint.
-void RenderScriptRuntime::PlaceBreakpointOnKernel(
- Stream &strm, const char *name, const std::array<int, 3> coords,
- Error &error, TargetSP target) {
- if (!name) {
- error.SetErrorString("invalid kernel name");
- return;
- }
+bool RenderScriptRuntime::PlaceBreakpointOnKernel(TargetSP target,
+ Stream &messages,
+ const char *name,
+ const RSCoordinate *coord) {
+ if (!name)
+ return false;
InitSearchFilter(target);
ConstString kernel_name(name);
BreakpointSP bp = CreateKernelBreakpoint(kernel_name);
+ if (!bp)
+ return false;
// We have a conditional breakpoint on a specific coordinate
- if (coords[0] != -1) {
- strm.Printf("Conditional kernel breakpoint on coordinate %" PRId32
- ", %" PRId32 ", %" PRId32,
- coords[0], coords[1], coords[2]);
- strm.EOL();
-
- // Allocate memory for the baton, and copy over coordinate
- uint32_t *baton = new uint32_t[coords.size()];
- baton[0] = coords[0];
- baton[1] = coords[1];
- baton[2] = coords[2];
-
- // Create a callback that will be invoked every time the breakpoint is hit.
- // The baton object passed to the handler is the target coordinate we want
- // to break on.
- bp->SetCallback(KernelBreakpointHit, baton, true);
-
- // Store a shared pointer to the baton, so the memory will eventually be
- // cleaned up after destruction
- m_conditional_breaks[bp->GetID()] = std::shared_ptr<uint32_t>(baton);
- }
+ if (coord)
+ SetConditional(bp, messages, *coord);
+
+ bp->GetDescription(&messages, lldb::eDescriptionLevelInitial, false);
- if (bp)
- bp->GetDescription(&strm, lldb::eDescriptionLevelInitial, false);
+ return true;
}
void RenderScriptRuntime::DumpModules(Stream &strm) const {
@@ -3730,12 +3756,18 @@ public:
const int short_option = m_getopt_table[option_idx].val;
switch (short_option) {
- case 'c':
- if (!ParseCoordinate(option_arg))
+ case 'c': {
+ auto coord = RSCoordinate{};
+ if (!ParseCoordinate(option_arg, coord))
error.SetErrorStringWithFormat(
"Couldn't parse coordinate '%s', should be in format 'x,y,z'.",
option_arg);
+ else {
+ m_have_coord = true;
+ m_coord = coord;
+ }
break;
+ }
default:
error.SetErrorStringWithFormat("unrecognized option '%c'",
short_option);
@@ -3744,46 +3776,16 @@ public:
return error;
}
- // -c takes an argument of the form 'num[,num][,num]'.
- // Where 'id_cstr' is this argument with the whitespace trimmed.
- // Missing coordinates are defaulted to zero.
- bool ParseCoordinate(const char *id_cstr) {
- RegularExpression regex;
- RegularExpression::Match regex_match(3);
-
- llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
- bool matched = false;
- if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
- regex.Execute(id_ref, ®ex_match))
- matched = true;
- else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
- regex.Execute(id_ref, ®ex_match))
- matched = true;
- else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
- regex.Execute(id_ref, ®ex_match))
- matched = true;
- for (uint32_t i = 0; i < 3; i++) {
- std::string group;
- if (regex_match.GetMatchAtIndex(id_cstr, i + 1, group))
- m_coord[i] = (uint32_t)strtoul(group.c_str(), nullptr, 0);
- else
- m_coord[i] = 0;
- }
- return matched;
- }
-
void OptionParsingStarting(ExecutionContext *execution_context) override {
- // -1 means the -c option hasn't been set
- m_coord[0] = -1;
- m_coord[1] = -1;
- m_coord[2] = -1;
+ m_have_coord = false;
}
llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
return llvm::makeArrayRef(g_renderscript_kernel_bp_set_options);
}
- std::array<int, 3> m_coord;
+ RSCoordinate m_coord;
+ bool m_have_coord;
};
bool DoExecute(Args &command, CommandReturnObject &result) override {
@@ -3800,19 +3802,20 @@ public:
(RenderScriptRuntime *)m_exe_ctx.GetProcessPtr()->GetLanguageRuntime(
eLanguageTypeExtRenderScript);
- Error error;
- runtime->PlaceBreakpointOnKernel(
- result.GetOutputStream(), command.GetArgumentAtIndex(0),
- m_options.m_coord, error, m_exe_ctx.GetTargetSP());
-
- if (error.Success()) {
- result.AppendMessage("Breakpoint(s) created");
- result.SetStatus(eReturnStatusSuccessFinishResult);
- return true;
+ auto &outstream = result.GetOutputStream();
+ auto &target = m_exe_ctx.GetTargetSP();
+ auto name = command.GetArgumentAtIndex(0);
+ auto coord = m_options.m_have_coord ? &m_options.m_coord : nullptr;
+ if (!runtime->PlaceBreakpointOnKernel(target, outstream, name, coord)) {
+ result.SetStatus(eReturnStatusFailed);
+ result.AppendErrorWithFormat(
+ "Error: unable to set breakpoint on kernel '%s'", name);
+ return false;
}
- result.SetStatus(eReturnStatusFailed);
- result.AppendErrorWithFormat("Error: %s", error.AsCString());
- return false;
+
+ result.AppendMessage("Breakpoint(s) created");
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ return true;
}
private:
@@ -3887,14 +3890,13 @@ public:
~CommandObjectRenderScriptRuntimeKernelCoordinate() override = default;
bool DoExecute(Args &command, CommandReturnObject &result) override {
- RSCoordinate coord{}; // Zero initialize array
+ RSCoordinate coord{};
bool success = RenderScriptRuntime::GetKernelCoordinate(
coord, m_exe_ctx.GetThreadPtr());
Stream &stream = result.GetOutputStream();
if (success) {
- stream.Printf("Coordinate: (%" PRIu32 ", %" PRIu32 ", %" PRIu32 ")",
- coord[0], coord[1], coord[2]);
+ stream.Printf("Coordinate: " FMT_COORD, coord.x, coord.y, coord.z);
stream.EOL();
result.SetStatus(eReturnStatusSuccessFinishResult);
} else {
Modified: lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h?rev=283320&r1=283319&r2=283320&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h (original)
+++ lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h Wed Oct 5 09:34:52 2016
@@ -40,7 +40,15 @@ struct RSReductionDescriptor;
typedef std::shared_ptr<RSModuleDescriptor> RSModuleDescriptorSP;
typedef std::shared_ptr<RSGlobalDescriptor> RSGlobalDescriptorSP;
typedef std::shared_ptr<RSKernelDescriptor> RSKernelDescriptorSP;
-typedef std::array<uint32_t, 3> RSCoordinate;
+struct RSCoordinate {
+ uint32_t x, y, z;
+
+ RSCoordinate() : x(), y(), z(){};
+
+ bool operator==(const lldb_renderscript::RSCoordinate &rhs) {
+ return x == rhs.x && y == rhs.y && z == rhs.z;
+ }
+};
// Breakpoint Resolvers decide where a breakpoint is placed,
// so having our own allows us to limit the search scope to RS kernel modules.
@@ -235,9 +243,9 @@ public:
bool RecomputeAllAllocations(Stream &strm, StackFrame *frame_ptr);
- void PlaceBreakpointOnKernel(Stream &strm, const char *name,
- const std::array<int, 3> coords, Error &error,
- lldb::TargetSP target);
+ bool PlaceBreakpointOnKernel(
+ lldb::TargetSP target, Stream &messages, const char *name,
+ const lldb_renderscript::RSCoordinate *coords = nullptr);
void SetBreakAllKernels(bool do_break, lldb::TargetSP target);
@@ -322,7 +330,8 @@ protected:
std::map<lldb::addr_t, lldb_renderscript::RSModuleDescriptorSP>
m_scriptMappings;
std::map<lldb::addr_t, RuntimeHookSP> m_runtimeHooks;
- std::map<lldb::user_id_t, std::shared_ptr<uint32_t>> m_conditional_breaks;
+ std::map<lldb::user_id_t, std::unique_ptr<lldb_renderscript::RSCoordinate>>
+ m_conditional_breaks;
lldb::SearchFilterSP
m_filtersp; // Needed to create breakpoints through Target API
@@ -376,6 +385,8 @@ private:
size_t CalculateElementHeaderSize(const Element &elem);
+ void SetConditional(lldb::BreakpointSP bp, lldb_private::Stream &messages,
+ const lldb_renderscript::RSCoordinate &coord);
//
// Helper functions for jitting the runtime
//
More information about the lldb-commits
mailing list