diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h --- a/lldb/include/lldb/Target/Trace.h +++ b/lldb/include/lldb/Target/Trace.h @@ -111,7 +111,7 @@ /// \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 --- a/lldb/include/lldb/Target/TraceSettingsParser.h +++ b/lldb/include/lldb/Target/TraceSettingsParser.h @@ -23,6 +23,38 @@ /// 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 file; + JSONAddress load_address; + llvm::Optional uuid; + }; + + struct JSONThread { + int64_t tid; + std::string trace_file; + }; + + struct JSONProcess { + int64_t pid; + std::string triple; + std::vector threads; + std::vector modules; + }; + + struct JSONTracePluginSettings { + std::string type; + }; + + struct JSONTraceSettings { + std::vector processes; + JSONTracePluginSettings trace; + }; + TraceSettingsParser(Trace &trace) : m_trace(trace) {} virtual ~TraceSettingsParser() = default; @@ -46,7 +78,7 @@ /// \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 @@ /// 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 @@ } // 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 ToIntegerOrError(const llvm::json::Value &value); - -llvm::Expected ToStringOrError(const llvm::json::Value &value); - -llvm::Expected -ToArrayOrError(const llvm::json::Value &value); - -llvm::Expected -ToObjectOrError(const llvm::json::Value &value); - -llvm::Error CreateMissingKeyError(llvm::json::Object obj, llvm::StringRef key); - -llvm::Expected -GetValueOrError(const llvm::json::Object &obj, llvm::StringRef key); - -llvm::Expected GetIntegerOrError(const llvm::json::Object &obj, - llvm::StringRef key); - -llvm::Expected GetStringOrError(const llvm::json::Object &obj, - llvm::StringRef key); - -llvm::Expected -GetArrayOrError(const llvm::json::Object &obj, llvm::StringRef key); - -llvm::Expected -GetObjectOrError(const llvm::json::Object &obj, llvm::StringRef key); - -llvm::Expected> -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 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.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h --- 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 @@ 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/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp @@ -24,45 +24,21 @@ })"; } -llvm::Error TraceIntelPTSettingsParser::ParsePTCPU(const json::Object &trace) { - llvm::Expected pt_cpu = - json_helpers::GetObjectOrError(trace, "pt_cpu"); - if (!pt_cpu) - return pt_cpu.takeError(); - - llvm::Expected vendor = - json_helpers::GetStringOrError(*pt_cpu, "vendor"); - if (!vendor) - return vendor.takeError(); - - llvm::Expected family = - json_helpers::GetIntegerOrError(*pt_cpu, "family"); - if (!family) - return family.takeError(); - - llvm::Expected model = - json_helpers::GetIntegerOrError(*pt_cpu, "model"); - if (!model) - return model.takeError(); - - llvm::Expected 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(*family), static_cast(*model), - static_cast(*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(pt_cpu.family), + static_cast(pt_cpu.model), + static_cast(pt_cpu.stepping)}; } -llvm::Error TraceIntelPTSettingsParser::ParsePluginSettings() { - llvm::Expected 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/Target/Trace.cpp b/lldb/source/Target/Trace.cpp --- a/lldb/source/Target/Trace.cpp +++ b/lldb/source/Target/Trace.cpp @@ -18,30 +18,48 @@ 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 Trace::FindPlugin(Debugger &debugger, const json::Value &settings, StringRef info_dir) { - llvm::Expected settings_obj = - json_helpers::ToObjectOrError(settings); - if (!settings_obj) - return settings_obj.takeError(); - - llvm::Expected trace = - json_helpers::GetObjectOrError(*settings_obj, "trace"); - if (!trace) - return trace.takeError(); - - llvm::Expected 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::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 --- a/lldb/source/Target/TraceSettingsParser.cpp +++ b/lldb/source/Target/TraceSettingsParser.cpp @@ -18,105 +18,6 @@ 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 ToIntegerOrError(const json::Value &value) { - llvm::Optional v = value.getAsInteger(); - if (v.hasValue()) - return *v; - return CreateWrongTypeError(value, "integer"); -} - -llvm::Expected ToStringOrError(const json::Value &value) { - llvm::Optional v = value.getAsString(); - if (v.hasValue()) - return *v; - return CreateWrongTypeError(value, "string"); -} - -llvm::Expected ToArrayOrError(const json::Value &value) { - if (const json::Array *v = value.getAsArray()) - return *v; - return CreateWrongTypeError(value, "array"); -} - -llvm::Expected 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 GetValueOrError(const json::Object &obj, - StringRef key) { - if (const json::Value *v = obj.get(key)) - return *v; - else - return CreateMissingKeyError(obj, key); -} - -llvm::Expected GetIntegerOrError(const json::Object &obj, - StringRef key) { - if (llvm::Expected v = GetValueOrError(obj, key)) - return ToIntegerOrError(*v); - else - return v.takeError(); -} - -llvm::Expected GetStringOrError(const json::Object &obj, - StringRef key) { - if (llvm::Expected v = GetValueOrError(obj, key)) - return ToStringOrError(*v); - else - return v.takeError(); -} - -llvm::Expected GetArrayOrError(const json::Object &obj, - StringRef key) { - if (llvm::Expected v = GetValueOrError(obj, key)) - return ToArrayOrError(*v); - else - return v.takeError(); -} - -llvm::Expected GetObjectOrError(const json::Object &obj, - StringRef key) { - if (llvm::Expected v = GetValueOrError(obj, key)) - return ToObjectOrError(*v); - else - return v.takeError(); -} - -llvm::Expected> -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 @@ file_spec.PrependPathComponent(m_settings_dir); } -llvm::Error TraceSettingsParser::ParseThread(ProcessSP &process_sp, - const json::Object &thread) { - llvm::Expected raw_tid = - json_helpers::GetIntegerOrError(thread, "tid"); - if (!raw_tid) - return raw_tid.takeError(); - lldb::tid_t tid = static_cast(*raw_tid); +void TraceSettingsParser::ParseThread(ProcessSP &process_sp, + const JSONThread &thread) { + lldb::tid_t tid = static_cast(thread.tid); - if (llvm::Expected 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 threads = - json_helpers::GetArrayOrError(process, "threads"); - if (!threads) - return threads.takeError(); - - for (const json::Value &thread_val : *threads) { - llvm::Expected 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 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 load_address_str = - json_helpers::GetStringOrError(module, "loadAddress"); - if (!load_address_str) - return load_address_str.takeError(); - llvm::Expected load_address = ParseAddress(*load_address_str); - if (!load_address) - return load_address.takeError(); - - llvm::Expected 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> 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> 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 @@ return error.ToError(); } -llvm::Error TraceSettingsParser::ParseModules(TargetSP &target_sp, - const json::Object &process) { - llvm::Expected modules = - json_helpers::GetArrayOrError(process, "modules"); - if (!modules) - return modules.takeError(); - - for (const json::Value &module_val : *modules) { - llvm::Expected 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 pid = json_helpers::GetIntegerOrError(process, "pid"); - if (!pid) - return pid.takeError(); - - llvm::Expected 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 @@ ProcessSP process_sp(target_sp->CreateProcess( /*listener*/ nullptr, /*plugin_name*/ llvm::StringRef(), /*crash_file*/ nullptr)); - process_sp->SetID(static_cast(*pid)); + process_sp->SetID(static_cast(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 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 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 --- a/lldb/test/API/commands/trace/TestTraceLoad.py +++ b/lldb/test/API/commands/trace/TestTraceLoad.py @@ -41,17 +41,43 @@ # 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"]) + substrs=['''error: expected object at settings.processes[0] - # Now we test a missing field +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 + 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: missing value at settings.processes[1].triple', "Context", "Schema"]) + + # Now we test a missing field in the intel-pt settings 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"]) + substrs=['error: missing value at settings.processes[1].triple', "Context", "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.']) + 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 --- /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": [ + ] +}