[Lldb-commits] [lldb] bddebca - [intel-pt] Refactor the JSON parsing
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 24 16:35:41 PDT 2020
Author: Walter Erquinigo
Date: 2020-09-24T16:35:34-07:00
New Revision: bddebca61ea73d45d5eafc38aaf9fe7605f5a9b3
URL: https://github.com/llvm/llvm-project/commit/bddebca61ea73d45d5eafc38aaf9fe7605f5a9b3
DIFF: https://github.com/llvm/llvm-project/commit/bddebca61ea73d45d5eafc38aaf9fe7605f5a9b3.diff
LOG: [intel-pt] Refactor the JSON parsing
Recently https://reviews.llvm.org/D88103 introduced a nice API for
converting a JSON object into C++ types, which include nice error
messaging.
I'm using that new functioniality to perform the parsing in a much more
elegant way. As a result, the code looks simpler and more maintainable,
as we aren't parsing anymore individual fields manually.
I updated the test cases accordingly.
Differential Revision: https://reviews.llvm.org/D88264
Added:
lldb/test/API/commands/trace/intelpt-trace/trace_bad4.json
lldb/test/API/commands/trace/intelpt-trace/trace_bad5.json
Modified:
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Target/TraceSettingsParser.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
lldb/source/Target/Trace.cpp
lldb/source/Target/TraceSettingsParser.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h
index 2328fba822a2..e4e9b1aa88a7 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -111,7 +111,7 @@ class Trace : public PluginInterface {
/// \return
/// An error object containing the reason if there is a failure.
llvm::Error ParseSettings(Debugger &debugger,
- const llvm::json::Object &settings,
+ const llvm::json::Value &settings,
llvm::StringRef settings_dir);
/// Get the JSON schema of the settings for the trace plug-in.
diff --git a/lldb/include/lldb/Target/TraceSettingsParser.h b/lldb/include/lldb/Target/TraceSettingsParser.h
index bc18c107ed83..47e6556e1273 100644
--- a/lldb/include/lldb/Target/TraceSettingsParser.h
+++ b/lldb/include/lldb/Target/TraceSettingsParser.h
@@ -23,6 +23,38 @@ namespace lldb_private {
/// overriden and implement the plug-in specific parsing logic.
class TraceSettingsParser {
public:
+ struct JSONAddress {
+ lldb::addr_t value;
+ };
+
+ struct JSONModule {
+ std::string system_path;
+ llvm::Optional<std::string> file;
+ JSONAddress load_address;
+ llvm::Optional<std::string> uuid;
+ };
+
+ struct JSONThread {
+ int64_t tid;
+ std::string trace_file;
+ };
+
+ struct JSONProcess {
+ int64_t pid;
+ std::string triple;
+ std::vector<JSONThread> threads;
+ std::vector<JSONModule> modules;
+ };
+
+ struct JSONTracePluginSettings {
+ std::string type;
+ };
+
+ struct JSONTraceSettings {
+ std::vector<JSONProcess> processes;
+ JSONTracePluginSettings trace;
+ };
+
TraceSettingsParser(Trace &trace) : m_trace(trace) {}
virtual ~TraceSettingsParser() = default;
@@ -46,7 +78,7 @@ class TraceSettingsParser {
/// \return
/// An error object containing the reason if there is a failure.
llvm::Error ParseSettings(Debugger &debugger,
- const llvm::json::Object &settings,
+ const llvm::json::Value &settings,
llvm::StringRef settings_dir);
protected:
@@ -57,33 +89,44 @@ class TraceSettingsParser {
/// Method that should be overriden to parse the plug-in specific settings.
///
+ /// \param[in] plugin_settings
+ /// The settings to parse specific to the plugin.
+ ///
/// \return
/// An error object containing the reason if there is a failure.
- virtual llvm::Error ParsePluginSettings() = 0;
+ virtual llvm::Error
+ ParsePluginSettings(const llvm::json::Value &plugin_settings) = 0;
+
+ /// Create a user-friendly error message upon a JSON-parsing failure using the
+ /// \a json::ObjectMapper functionality.
+ ///
+ /// \param[in] root
+ /// The \a llvm::json::Path::Root used to parse the JSON \a value.
+ ///
+ /// \param[in] value
+ /// The json value that failed to parse.
+ ///
+ /// \return
+ /// An \a llvm::Error containing the user-friendly error message.
+ llvm::Error CreateJSONError(llvm::json::Path::Root &root,
+ const llvm::json::Value &value);
private:
- /// Resolve non-absolute paths relativejto the settings folder
+ /// Resolve non-absolute paths relativeto the settings folder
void NormalizePath(lldb_private::FileSpec &file_spec);
+
llvm::Error ParseProcess(lldb_private::Debugger &debugger,
- const llvm::json::Object &process);
- llvm::Error ParseProcesses(lldb_private::Debugger &debugger);
- llvm::Error ParseThread(lldb::ProcessSP &process_sp,
- const llvm::json::Object &thread);
- llvm::Error ParseThreads(lldb::ProcessSP &process_sp,
- const llvm::json::Object &process);
- llvm::Error ParseModule(lldb::TargetSP &target_sp,
- const llvm::json::Object &module);
- llvm::Error ParseModules(lldb::TargetSP &target_sp,
- const llvm::json::Object &process);
- llvm::Error ParseSettingsImpl(lldb_private::Debugger &debugger);
+ const JSONProcess &process);
+ void ParseThread(lldb::ProcessSP &process_sp, const JSONThread &thread);
+ llvm::Error ParseModule(lldb::TargetSP &target_sp, const JSONModule &module);
+ llvm::Error ParseSettingsImpl(lldb_private::Debugger &debugger,
+ const llvm::json::Value &settings);
Trace &m_trace;
protected:
/// Objects created as product of the parsing
/// \{
- /// JSON object that holds all settings for this trace session.
- llvm::json::Object m_settings;
/// The directory that contains the settings file.
std::string m_settings_dir;
@@ -95,42 +138,63 @@ class TraceSettingsParser {
} // namespace lldb_private
-namespace json_helpers {
-/// JSON parsing helpers based on \a llvm::Expected.
-/// \{
-llvm::Error CreateWrongTypeError(const llvm::json::Value &value,
- llvm::StringRef type);
-
-llvm::Expected<int64_t> ToIntegerOrError(const llvm::json::Value &value);
-
-llvm::Expected<llvm::StringRef> ToStringOrError(const llvm::json::Value &value);
-
-llvm::Expected<const llvm::json::Array &>
-ToArrayOrError(const llvm::json::Value &value);
-
-llvm::Expected<const llvm::json::Object &>
-ToObjectOrError(const llvm::json::Value &value);
-
-llvm::Error CreateMissingKeyError(llvm::json::Object obj, llvm::StringRef key);
-
-llvm::Expected<const llvm::json::Value &>
-GetValueOrError(const llvm::json::Object &obj, llvm::StringRef key);
-
-llvm::Expected<int64_t> GetIntegerOrError(const llvm::json::Object &obj,
- llvm::StringRef key);
-
-llvm::Expected<llvm::StringRef> GetStringOrError(const llvm::json::Object &obj,
- llvm::StringRef key);
-
-llvm::Expected<const llvm::json::Array &>
-GetArrayOrError(const llvm::json::Object &obj, llvm::StringRef key);
-
-llvm::Expected<const llvm::json::Object &>
-GetObjectOrError(const llvm::json::Object &obj, llvm::StringRef key);
-
-llvm::Expected<llvm::Optional<llvm::StringRef>>
-GetOptionalStringOrError(const llvm::json::Object &obj, llvm::StringRef key);
-/// \}
-} // namespace json_helpers
+namespace llvm {
+namespace json {
+
+inline bool fromJSON(const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONAddress &address,
+ llvm::json::Path path) {
+ llvm::Optional<llvm::StringRef> s = value.getAsString();
+ if (s.hasValue() && !s->getAsInteger(0, address.value))
+ return true;
+
+ path.report("expected numeric string");
+ return false;
+}
+
+inline bool fromJSON(const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONModule &module,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("systemPath", module.system_path) &&
+ o.map("file", module.file) &&
+ o.map("loadAddress", module.load_address) &&
+ o.map("uuid", module.uuid);
+}
+
+inline bool fromJSON(const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONThread &thread,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("tid", thread.tid) && o.map("traceFile", thread.trace_file);
+}
+
+inline bool fromJSON(const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONProcess &process,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("pid", process.pid) && o.map("triple", process.triple) &&
+ o.map("threads", process.threads) && o.map("modules", process.modules);
+}
+
+inline bool fromJSON(
+ const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONTracePluginSettings &plugin_settings,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("type", plugin_settings.type);
+}
+
+inline bool
+fromJSON(const llvm::json::Value &value,
+ lldb_private::TraceSettingsParser::JSONTraceSettings &settings,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("trace", settings.trace) &&
+ o.map("processes", settings.processes);
+}
+
+} // namespace json
+} // namespace llvm
#endif // LLDB_TARGET_TRACE_SETTINGS_PARSER_H
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
index 2430f779d956..c67822d49155 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
@@ -24,45 +24,21 @@ StringRef TraceIntelPTSettingsParser::GetPluginSchema() {
})";
}
-llvm::Error TraceIntelPTSettingsParser::ParsePTCPU(const json::Object &trace) {
- llvm::Expected<const json::Object &> pt_cpu =
- json_helpers::GetObjectOrError(trace, "pt_cpu");
- if (!pt_cpu)
- return pt_cpu.takeError();
-
- llvm::Expected<llvm::StringRef> vendor =
- json_helpers::GetStringOrError(*pt_cpu, "vendor");
- if (!vendor)
- return vendor.takeError();
-
- llvm::Expected<int64_t> family =
- json_helpers::GetIntegerOrError(*pt_cpu, "family");
- if (!family)
- return family.takeError();
-
- llvm::Expected<int64_t> model =
- json_helpers::GetIntegerOrError(*pt_cpu, "model");
- if (!model)
- return model.takeError();
-
- llvm::Expected<int64_t> stepping =
- json_helpers::GetIntegerOrError(*pt_cpu, "stepping");
- if (!stepping)
- return stepping.takeError();
-
- m_pt_cpu = {vendor->compare("intel") == 0 ? pcv_intel : pcv_unknown,
- static_cast<uint16_t>(*family), static_cast<uint8_t>(*model),
- static_cast<uint8_t>(*stepping)};
- return llvm::Error::success();
+void TraceIntelPTSettingsParser::ParsePTCPU(const JSONPTCPU &pt_cpu) {
+ m_pt_cpu = {pt_cpu.vendor.compare("intel") == 0 ? pcv_intel : pcv_unknown,
+ static_cast<uint16_t>(pt_cpu.family),
+ static_cast<uint8_t>(pt_cpu.model),
+ static_cast<uint8_t>(pt_cpu.stepping)};
}
-llvm::Error TraceIntelPTSettingsParser::ParsePluginSettings() {
- llvm::Expected<const json::Object &> trace =
- json_helpers::GetObjectOrError(m_settings, "trace");
- if (!trace)
- return trace.takeError();
- if (llvm::Error err = ParsePTCPU(*trace))
- return err;
+llvm::Error TraceIntelPTSettingsParser::ParsePluginSettings(
+ const llvm::json::Value &plugin_settings) {
+ json::Path::Root root("settings.trace");
+ JSONIntelPTSettings settings;
+ if (!json::fromJSON(plugin_settings, settings, root))
+ return CreateJSONError(root, plugin_settings);
+
+ ParsePTCPU(settings.pt_cpu);
m_trace.m_pt_cpu = m_pt_cpu;
return llvm::Error::success();
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
index b8aaa106b10d..8c9b9ea89829 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
@@ -18,8 +18,18 @@
class TraceIntelPT;
class TraceIntelPTSettingsParser : public lldb_private::TraceSettingsParser {
-
public:
+ struct JSONPTCPU {
+ std::string vendor;
+ int64_t family;
+ int64_t model;
+ int64_t stepping;
+ };
+
+ struct JSONIntelPTSettings {
+ JSONPTCPU pt_cpu;
+ };
+
TraceIntelPTSettingsParser(TraceIntelPT &trace)
: lldb_private::TraceSettingsParser((lldb_private::Trace &)trace),
m_trace(trace) {}
@@ -27,13 +37,37 @@ class TraceIntelPTSettingsParser : public lldb_private::TraceSettingsParser {
protected:
llvm::StringRef GetPluginSchema() override;
- llvm::Error ParsePluginSettings() override;
+ llvm::Error
+ ParsePluginSettings(const llvm::json::Value &plugin_settings) override;
private:
- llvm::Error ParsePTCPU(const llvm::json::Object &trace);
+ void ParsePTCPU(const JSONPTCPU &pt_cpu);
TraceIntelPT &m_trace;
pt_cpu m_pt_cpu;
};
+namespace llvm {
+namespace json {
+
+inline bool fromJSON(const llvm::json::Value &value,
+ TraceIntelPTSettingsParser::JSONPTCPU &pt_cpu,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("vendor", pt_cpu.vendor) &&
+ o.map("family", pt_cpu.family) && o.map("model", pt_cpu.model) &&
+ o.map("stepping", pt_cpu.stepping);
+}
+
+inline bool
+fromJSON(const llvm::json::Value &value,
+ TraceIntelPTSettingsParser::JSONIntelPTSettings &intel_pt_settings,
+ llvm::json::Path path) {
+ llvm::json::ObjectMapper o(value, path);
+ return o && o.map("pt_cpu", intel_pt_settings.pt_cpu);
+}
+
+} // namespace json
+} // namespace llvm
+
#endif // liblldb_TraceIntelPTSettingsParser_h_
diff --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp
index afe4ee20be48..8d305c9886ca 100644
--- a/lldb/source/Target/Trace.cpp
+++ b/lldb/source/Target/Trace.cpp
@@ -18,30 +18,48 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm;
+// Helper structs used to extract the type of a trace settings json without
+// having to parse the entire object.
+
+struct JSONSimplePluginSettings {
+ std::string type;
+};
+
+struct JSONSimpleTraceSettings {
+ JSONSimplePluginSettings trace;
+};
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const json::Value &value,
+ JSONSimplePluginSettings &plugin_settings, json::Path path) {
+ json::ObjectMapper o(value, path);
+ return o && o.map("type", plugin_settings.type);
+}
+
+bool fromJSON(const json::Value &value, JSONSimpleTraceSettings &settings,
+ json::Path path) {
+ json::ObjectMapper o(value, path);
+ return o && o.map("trace", settings.trace);
+}
+
+} // namespace json
+} // namespace llvm
+
llvm::Expected<lldb::TraceSP> Trace::FindPlugin(Debugger &debugger,
const json::Value &settings,
StringRef info_dir) {
- llvm::Expected<const json::Object &> settings_obj =
- json_helpers::ToObjectOrError(settings);
- if (!settings_obj)
- return settings_obj.takeError();
-
- llvm::Expected<const json::Object &> trace =
- json_helpers::GetObjectOrError(*settings_obj, "trace");
- if (!trace)
- return trace.takeError();
-
- llvm::Expected<StringRef> type =
- json_helpers::GetStringOrError(*trace, "type");
- if (!type)
- return type.takeError();
-
- ConstString plugin_name(*type);
+ JSONSimpleTraceSettings json_settings;
+ json::Path::Root root("settings");
+ if (!json::fromJSON(settings, json_settings, root))
+ return root.getError();
+
+ ConstString plugin_name(json_settings.trace.type);
auto create_callback = PluginManager::GetTraceCreateCallback(plugin_name);
if (create_callback) {
TraceSP instance = create_callback();
- if (llvm::Error err =
- instance->ParseSettings(debugger, *settings_obj, info_dir))
+ if (llvm::Error err = instance->ParseSettings(debugger, settings, info_dir))
return std::move(err);
return instance;
}
@@ -65,7 +83,7 @@ llvm::Expected<lldb::TraceSP> Trace::FindPlugin(StringRef name) {
}
llvm::Error Trace::ParseSettings(Debugger &debugger,
- const llvm::json::Object &settings,
+ const llvm::json::Value &settings,
llvm::StringRef settings_dir) {
if (llvm::Error err =
CreateParser()->ParseSettings(debugger, settings, settings_dir))
diff --git a/lldb/source/Target/TraceSettingsParser.cpp b/lldb/source/Target/TraceSettingsParser.cpp
index 9021d93e17e0..cc3369fabcbf 100644
--- a/lldb/source/Target/TraceSettingsParser.cpp
+++ b/lldb/source/Target/TraceSettingsParser.cpp
@@ -18,105 +18,6 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm;
-namespace json_helpers {
-
-llvm::Error CreateWrongTypeError(const json::Value &value,
- llvm::StringRef type) {
- std::string s;
- llvm::raw_string_ostream os(s);
- os << llvm::formatv("JSON value is expected to be \"{0}\".\nValue:\n{1:2}",
- type, value);
- os.flush();
-
- return llvm::createStringError(std::errc::invalid_argument, os.str().c_str());
-}
-
-llvm::Expected<int64_t> ToIntegerOrError(const json::Value &value) {
- llvm::Optional<int64_t> v = value.getAsInteger();
- if (v.hasValue())
- return *v;
- return CreateWrongTypeError(value, "integer");
-}
-
-llvm::Expected<StringRef> ToStringOrError(const json::Value &value) {
- llvm::Optional<StringRef> v = value.getAsString();
- if (v.hasValue())
- return *v;
- return CreateWrongTypeError(value, "string");
-}
-
-llvm::Expected<const json::Array &> ToArrayOrError(const json::Value &value) {
- if (const json::Array *v = value.getAsArray())
- return *v;
- return CreateWrongTypeError(value, "array");
-}
-
-llvm::Expected<const json::Object &> ToObjectOrError(const json::Value &value) {
- if (const json::Object *v = value.getAsObject())
- return *v;
- return CreateWrongTypeError(value, "object");
-}
-
-llvm::Error CreateMissingKeyError(json::Object obj, llvm::StringRef key) {
- std::string str;
- llvm::raw_string_ostream os(str);
- os << llvm::formatv(
- "JSON object is missing the \"{0}\" field.\nValue:\n{1:2}", key,
- json::Value(std::move(obj)));
- os.flush();
-
- return llvm::createStringError(std::errc::invalid_argument, os.str().c_str());
-}
-
-llvm::Expected<const json::Value &> GetValueOrError(const json::Object &obj,
- StringRef key) {
- if (const json::Value *v = obj.get(key))
- return *v;
- else
- return CreateMissingKeyError(obj, key);
-}
-
-llvm::Expected<int64_t> GetIntegerOrError(const json::Object &obj,
- StringRef key) {
- if (llvm::Expected<const json::Value &> v = GetValueOrError(obj, key))
- return ToIntegerOrError(*v);
- else
- return v.takeError();
-}
-
-llvm::Expected<StringRef> GetStringOrError(const json::Object &obj,
- StringRef key) {
- if (llvm::Expected<const json::Value &> v = GetValueOrError(obj, key))
- return ToStringOrError(*v);
- else
- return v.takeError();
-}
-
-llvm::Expected<const json::Array &> GetArrayOrError(const json::Object &obj,
- StringRef key) {
- if (llvm::Expected<const json::Value &> v = GetValueOrError(obj, key))
- return ToArrayOrError(*v);
- else
- return v.takeError();
-}
-
-llvm::Expected<const json::Object &> GetObjectOrError(const json::Object &obj,
- StringRef key) {
- if (llvm::Expected<const json::Value &> v = GetValueOrError(obj, key))
- return ToObjectOrError(*v);
- else
- return v.takeError();
-}
-
-llvm::Expected<llvm::Optional<StringRef>>
-GetOptionalStringOrError(const json::Object &obj, StringRef key) {
- if (const json::Value *v = obj.get(key))
- return ToStringOrError(*v);
- return llvm::None;
-}
-
-} // namespace json_helpers
-
StringRef TraceSettingsParser::GetSchema() {
static std::string schema;
if (schema.empty()) {
@@ -156,89 +57,34 @@ void TraceSettingsParser::NormalizePath(FileSpec &file_spec) {
file_spec.PrependPathComponent(m_settings_dir);
}
-llvm::Error TraceSettingsParser::ParseThread(ProcessSP &process_sp,
- const json::Object &thread) {
- llvm::Expected<lldb::tid_t> raw_tid =
- json_helpers::GetIntegerOrError(thread, "tid");
- if (!raw_tid)
- return raw_tid.takeError();
- lldb::tid_t tid = static_cast<lldb::tid_t>(*raw_tid);
+void TraceSettingsParser::ParseThread(ProcessSP &process_sp,
+ const JSONThread &thread) {
+ lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid);
- if (llvm::Expected<StringRef> trace_file =
- json_helpers::GetStringOrError(thread, "traceFile")) {
- FileSpec spec(*trace_file);
- NormalizePath(spec);
- m_thread_to_trace_file_map[process_sp->GetID()][tid] = spec;
- } else
- return trace_file.takeError();
+ FileSpec spec(thread.trace_file);
+ NormalizePath(spec);
+ m_thread_to_trace_file_map[process_sp->GetID()][tid] = spec;
ThreadSP thread_sp(new HistoryThread(*process_sp, tid, /*callstack*/ {}));
process_sp->GetThreadList().AddThread(thread_sp);
- return llvm::Error::success();
-}
-
-llvm::Error TraceSettingsParser::ParseThreads(ProcessSP &process_sp,
- const json::Object &process) {
- llvm::Expected<const json::Array &> threads =
- json_helpers::GetArrayOrError(process, "threads");
- if (!threads)
- return threads.takeError();
-
- for (const json::Value &thread_val : *threads) {
- llvm::Expected<const json::Object &> thread =
- json_helpers::ToObjectOrError(thread_val);
- if (!thread)
- return thread.takeError();
- if (llvm::Error err = ParseThread(process_sp, *thread))
- return err;
- }
- return llvm::Error::success();
-}
-
-static llvm::Expected<addr_t> ParseAddress(StringRef address_str) {
- addr_t address;
- if (address_str.getAsInteger(0, address))
- return createStringError(std::errc::invalid_argument,
- "\"%s\" does not represent an integer",
- address_str.data());
- return address;
}
llvm::Error TraceSettingsParser::ParseModule(TargetSP &target_sp,
- const json::Object &module) {
- llvm::Expected<StringRef> load_address_str =
- json_helpers::GetStringOrError(module, "loadAddress");
- if (!load_address_str)
- return load_address_str.takeError();
- llvm::Expected<addr_t> load_address = ParseAddress(*load_address_str);
- if (!load_address)
- return load_address.takeError();
-
- llvm::Expected<StringRef> system_path =
- json_helpers::GetStringOrError(module, "systemPath");
- if (!system_path)
- return system_path.takeError();
- FileSpec system_file_spec(*system_path);
+ const JSONModule &module) {
+ FileSpec system_file_spec(module.system_path);
NormalizePath(system_file_spec);
- llvm::Expected<llvm::Optional<StringRef>> file =
- json_helpers::GetOptionalStringOrError(module, "file");
- if (!file)
- return file.takeError();
- FileSpec local_file_spec(file->hasValue() ? **file : *system_path);
+ FileSpec local_file_spec(module.file.hasValue() ? *module.file
+ : module.system_path);
NormalizePath(local_file_spec);
ModuleSpec module_spec;
module_spec.GetFileSpec() = local_file_spec;
module_spec.GetPlatformFileSpec() = system_file_spec;
- module_spec.SetObjectOffset(*load_address);
+ module_spec.SetObjectOffset(module.load_address.value);
- llvm::Expected<llvm::Optional<StringRef>> uuid_str =
- json_helpers::GetOptionalStringOrError(module, "uuid");
- if (!uuid_str)
- return uuid_str.takeError();
- if (uuid_str->hasValue())
- module_spec.GetUUID().SetFromStringRef(**uuid_str);
+ if (module.uuid.hasValue())
+ module_spec.GetUUID().SetFromStringRef(*module.uuid);
Status error;
ModuleSP module_sp =
@@ -246,38 +92,12 @@ llvm::Error TraceSettingsParser::ParseModule(TargetSP &target_sp,
return error.ToError();
}
-llvm::Error TraceSettingsParser::ParseModules(TargetSP &target_sp,
- const json::Object &process) {
- llvm::Expected<const json::Array &> modules =
- json_helpers::GetArrayOrError(process, "modules");
- if (!modules)
- return modules.takeError();
-
- for (const json::Value &module_val : *modules) {
- llvm::Expected<const json::Object &> module =
- json_helpers::ToObjectOrError(module_val);
- if (!module)
- return module.takeError();
- if (llvm::Error err = ParseModule(target_sp, *module))
- return err;
- }
- return llvm::Error::success();
-}
-
llvm::Error TraceSettingsParser::ParseProcess(Debugger &debugger,
- const json::Object &process) {
- llvm::Expected<int64_t> pid = json_helpers::GetIntegerOrError(process, "pid");
- if (!pid)
- return pid.takeError();
-
- llvm::Expected<StringRef> triple =
- json_helpers::GetStringOrError(process, "triple");
- if (!triple)
- return triple.takeError();
-
+ const JSONProcess &process) {
TargetSP target_sp;
Status error = debugger.GetTargetList().CreateTarget(
- debugger, /*user_exe_path*/ llvm::StringRef(), *triple, eLoadDependentsNo,
+ debugger, /*user_exe_path*/ llvm::StringRef(), process.triple,
+ eLoadDependentsNo,
/*platform_options*/ nullptr, target_sp);
if (!target_sp)
@@ -289,55 +109,64 @@ llvm::Error TraceSettingsParser::ParseProcess(Debugger &debugger,
ProcessSP process_sp(target_sp->CreateProcess(
/*listener*/ nullptr, /*plugin_name*/ llvm::StringRef(),
/*crash_file*/ nullptr));
- process_sp->SetID(static_cast<lldb::pid_t>(*pid));
+ process_sp->SetID(static_cast<lldb::pid_t>(process.pid));
- if (llvm::Error err = ParseThreads(process_sp, process))
- return err;
+ for (const JSONThread &thread : process.threads)
+ ParseThread(process_sp, thread);
- return ParseModules(target_sp, process);
+ for (const JSONModule &module : process.modules) {
+ if (llvm::Error err = ParseModule(target_sp, module))
+ return err;
+ }
+ return llvm::Error::success();
}
-llvm::Error TraceSettingsParser::ParseProcesses(Debugger &debugger) {
- llvm::Expected<const json::Array &> processes =
- json_helpers::GetArrayOrError(m_settings, "processes");
- if (!processes)
- return processes.takeError();
+llvm::Error
+TraceSettingsParser::CreateJSONError(json::Path::Root &root,
+ const llvm::json::Value &value) {
+ std::string err;
+ raw_string_ostream os(err);
+ root.printErrorContext(value, os);
+ return createStringError(std::errc::invalid_argument,
+ "%s\n\nContext:\n%s\n\nSchema:\n%s",
+ llvm::toString(root.getError()).c_str(),
+ os.str().c_str(), GetSchema().data());
+}
- for (const json::Value &process_val : *processes) {
- llvm::Expected<const json::Object &> process =
- json_helpers::ToObjectOrError(process_val);
- if (!process)
- return process.takeError();
- if (llvm::Error err = ParseProcess(debugger, *process))
+llvm::Error
+TraceSettingsParser::ParseSettingsImpl(Debugger &debugger,
+ const llvm::json::Value &raw_settings) {
+ json::Path::Root root("settings");
+ JSONTraceSettings settings;
+ if (!json::fromJSON(raw_settings, settings, root))
+ return CreateJSONError(root, raw_settings);
+
+ for (const JSONProcess &process : settings.processes) {
+ if (llvm::Error err = ParseProcess(debugger, process))
return err;
}
- return llvm::Error::success();
-}
-llvm::Error TraceSettingsParser::ParseSettingsImpl(Debugger &debugger) {
- if (llvm::Error err = ParseProcesses(debugger))
- return err;
- return ParsePluginSettings();
+ json::Object plugin_obj = *raw_settings.getAsObject()->getObject("trace");
+ json::Value plugin_settings(std::move(plugin_obj));
+ return ParsePluginSettings(plugin_settings);
}
llvm::Error
TraceSettingsParser::ParseSettings(Debugger &debugger,
- const llvm::json::Object &settings,
+ const llvm::json::Value &raw_settings,
llvm::StringRef settings_dir) {
- m_settings = settings;
m_settings_dir = settings_dir.str();
- if (llvm::Error err = ParseSettingsImpl(debugger)) {
+
+ if (llvm::Error err = ParseSettingsImpl(debugger, raw_settings)) {
// We clean all the targets that were created internally, which should leave
// the debugger unchanged
for (auto target_sp : m_targets)
debugger.GetTargetList().DeleteTarget(target_sp);
- return createStringError(std::errc::invalid_argument, "%s\nSchema:\n%s",
- llvm::toString(std::move(err)).c_str(),
- GetSchema().data());
+ return err;
}
- m_trace.m_settings = m_settings;
+ m_trace.m_settings = *raw_settings.getAsObject();
m_trace.m_settings_dir = m_settings_dir;
m_trace.m_thread_to_trace_file_map = m_thread_to_trace_file_map;
m_trace.m_targets = m_targets;
diff --git a/lldb/test/API/commands/trace/TestTraceLoad.py b/lldb/test/API/commands/trace/TestTraceLoad.py
index f418b8d5a822..5747c136877a 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -39,19 +39,56 @@ def testLoadTrace(self):
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
# We test first an invalid type
- trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace_bad.json")
- self.expect("trace load -v " + trace_definition_file, error=True,
- substrs=['error: JSON value is expected to be "object"', "Value", "123", "Schema"])
+ self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad.json"), error=True,
+ substrs=['''error: expected object at settings.processes[0]
- # Now we test a missing field
- trace_definition_file2 = os.path.join(src_dir, "intelpt-trace", "trace_bad2.json")
- self.expect("trace load -v " + trace_definition_file2, error=True,
- substrs=['error: JSON object is missing the "triple" field.', "Value", "pid", "12345", "Schema"])
+Context:
+{
+ "processes": [
+ /* error: expected object */
+ 123
+ ],
+ "trace": { ... }
+}
+
+Schema:
+{
+ "trace": {
+ "type": "intel-pt",
+ "pt_cpu": {
+ "vendor": "intel" | "unknown",
+ "family": integer,
+ "model": integer,
+ "stepping": integer
+ }
+ },'''])
+
+ # Now we test a missing field in the global settings
+ self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad2.json"), error=True,
+ substrs=['error: missing value at settings.processes[1].triple', "Context", "Schema"])
+
+ # Now we test a missing field in the intel-pt settings
+ self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad4.json"), error=True,
+ substrs=['''error: missing value at settings.trace.pt_cpu.family
+
+Context:
+{
+ "pt_cpu": /* error: missing value */ {
+ "model": 79,
+ "stepping": 1,
+ "vendor": "intel"
+ },
+ "type": "intel-pt"
+}''', "Schema"])
+
+ # Now we test an incorrect load address in the intel-pt settings
+ self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad5.json"), error=True,
+ substrs=['error: expected numeric string at settings.processes[0].modules[0].loadAddress',
+ '"loadAddress": /* error: expected numeric string */ 400000,', "Schema"])
# The following wrong schema will have a valid target and an invalid one. In the case of failure,
# no targets should be created.
self.assertEqual(self.dbg.GetNumTargets(), 0)
- trace_definition_file2 = os.path.join(src_dir, "intelpt-trace", "trace_bad3.json")
- self.expect("trace load -v " + trace_definition_file2, error=True,
- substrs=['error: JSON object is missing the "pid" field.'])
+ self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad3.json"), error=True,
+ substrs=['error: missing value at settings.processes[1].pid'])
self.assertEqual(self.dbg.GetNumTargets(), 0)
diff --git a/lldb/test/API/commands/trace/intelpt-trace/trace_bad4.json b/lldb/test/API/commands/trace/intelpt-trace/trace_bad4.json
new file mode 100644
index 000000000000..08084829eda6
--- /dev/null
+++ b/lldb/test/API/commands/trace/intelpt-trace/trace_bad4.json
@@ -0,0 +1,12 @@
+{
+ "trace": {
+ "type": "intel-pt",
+ "pt_cpu": {
+ "vendor": "intel",
+ "model": 79,
+ "stepping": 1
+ }
+ },
+ "processes": [
+ ]
+}
diff --git a/lldb/test/API/commands/trace/intelpt-trace/trace_bad5.json b/lldb/test/API/commands/trace/intelpt-trace/trace_bad5.json
new file mode 100644
index 000000000000..b75db4355459
--- /dev/null
+++ b/lldb/test/API/commands/trace/intelpt-trace/trace_bad5.json
@@ -0,0 +1,32 @@
+{
+ "trace": {
+ "type": "intel-pt",
+ "pt_cpu": {
+ "vendor": "intel",
+ "family": 6,
+ "model": 79,
+ "stepping": 1
+ }
+ },
+ "processes": [
+ {
+ "pid": 1234,
+ "triple": "x86_64-*-linux",
+ "threads": [
+ {
+ "tid": 5678,
+ "traceFile": "3842849.trace"
+ }
+ ],
+ "modules": [
+ {
+ "file": "a.out",
+ "systemPath": "a.out",
+ "loadAddress": 400000,
+ "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+ }
+ ]
+ },
+ {}
+ ]
+}
More information about the lldb-commits
mailing list