[intel-pt] Refactor the JSON parsing

[wallace]

Summary

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.

lldb/include/lldb/Target/Trace.h

public:
   /// \param[in] settings_dir
   ///     Path to a directory used to resolve relative paths in the JSON data.
   ///     If the JSON data is defined in a file, this should be the
   ///     folder containing it.
   ///
   /// \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.
   llvm::StringRef GetSchema();
 
 protected:
   Trace() {}
 

lldb/include/lldb/Target/TraceSettingsParser.h

 
 /// \class TraceSettingsParser TraceSettingsParser.h
 /// A plug-in interface definition class for parsing \a Trace settings.
 ///
 /// As \a Trace plug-ins support plug-in specific settings, this class should be
 /// 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;
 
   /// Get the JSON schema of the settings for the trace plug-in.
   llvm::StringRef GetSchema();
 
   /// Parse the structured data settings and create the corresponding \a Target
   /// objects. In case of and error, no targets are created.
   ///
   /// \param[in] debugger
   ///   The debugger instance where the targets are created.
   ///
   /// \param[in] settings
   ///   The settings to parse.
   ///
   /// \param[in] settings_dir
   ///   The directory that contains the settings file used to resolve relative
   ///   paths.
   ///
   /// \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:
   /// Method that should be overriden by implementations of this class to
   /// provide the specific plug-in schema inside the "trace" section of the
   /// global schema.
   virtual llvm::StringRef GetPluginSchema() = 0;
 
   /// 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;
 
   std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>>
       m_thread_to_trace_file_map;
   std::vector<lldb::TargetSP> m_targets;
   /// \}
 };
 
 } // 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);
+namespace llvm {
+namespace json {
 
-llvm::Expected<const llvm::json::Object &>
-ToObjectOrError(const llvm::json::Value &value);
+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);
+}
 
-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 json
+} // namespace llvm
 
 #endif // LLDB_TARGET_TRACE_SETTINGS_PARSER_H

lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h

 
 #include "TraceIntelPT.h"
 #include "lldb/Target/TraceSettingsParser.h"
 #include "lldb/Utility/StructuredData.h"
 
 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) {}
 
 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_

lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp

"pt_cpu": {
       "vendor": "intel" | "unknown",
       "family": integer,
       "model": integer,
       "stepping": integer
     }
   })";
 }
 
-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();
 }

lldb/source/Target/Trace.cpp

 #include "llvm/Support/Format.h"
 
 #include "lldb/Core/PluginManager.h"
 
 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();
+  JSONSimpleTraceSettings json_settings;
+  json::Path::Root root("settings");
+  if (!json::fromJSON(settings, json_settings, root))
+    return root.getError();
 
-  ConstString plugin_name(*type);
+  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;
   }
 
   return createStringError(
       std::errc::invalid_argument,
       "no trace plug-in matches the specified type: \"%s\"",
       plugin_name.AsCString());
 }
 
 llvm::Expected<lldb::TraceSP> Trace::FindPlugin(StringRef name) {
   ConstString plugin_name(name);
   auto create_callback = PluginManager::GetTraceCreateCallback(plugin_name);
   if (create_callback)
     return create_callback();
 
   return createStringError(
       std::errc::invalid_argument,
       "no trace plug-in matches the specified type: \"%s\"",
       plugin_name.AsCString());
 }
 
 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))
     return err;
 
   return llvm::Error::success();
 }
 
 llvm::StringRef Trace::GetSchema() { return CreateParser()->GetSchema(); }

lldb/source/Target/TraceSettingsParser.cpp

 #include "Plugins/Process/Utility/HistoryThread.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Target/Process.h"
 
 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()) {
     std::ostringstream schema_builder;
     schema_builder << "{\n \"trace\": ";
     schema_builder << GetPluginSchema().str() << ",\n";
     schema_builder << R"(  "processes": [
     {
// All paths are either absolute or relative to the settings file.)";
   return schema;
 }
 
 void TraceSettingsParser::NormalizePath(FileSpec &file_spec) {
   if (file_spec.IsRelative())
     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);
-
-  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();
+void TraceSettingsParser::ParseThread(ProcessSP &process_sp,
+                                      const JSONThread &thread) {
+  lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid);
+
+  FileSpec spec(thread.trace_file);
 
   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 =
       target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error);
   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)
     return error.ToError();
 
   m_targets.push_back(target_sp);
   debugger.GetTargetList().SetSelectedTarget(target_sp.get());
 
   ProcessSP process_sp(target_sp->CreateProcess(
       /*listener*/ nullptr, /*plugin_name*/ llvm::StringRef(),
       /*crash_file*/ nullptr));
-  process_sp->SetID(static_cast<lldb::pid_t>(*pid));
-
-  if (llvm::Error err = ParseThreads(process_sp, process))
-    return err;
+  process_sp->SetID(static_cast<lldb::pid_t>(process.pid));
 
-  return ParseModules(target_sp, process);
-}
+  for (const JSONThread &thread : process.threads)
+    ParseThread(process_sp, thread);
 
-llvm::Error TraceSettingsParser::ParseProcesses(Debugger &debugger) {
-  llvm::Expected<const json::Array &> processes =
-      json_helpers::GetArrayOrError(m_settings, "processes");
-  if (!processes)
-    return processes.takeError();
-
-  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))
+  for (const JSONModule &module : process.modules) {
+    if (llvm::Error err = ParseModule(target_sp, module))
       return err;
   }
   return llvm::Error::success();
 }
 
-llvm::Error TraceSettingsParser::ParseSettingsImpl(Debugger &debugger) {
-  if (llvm::Error err = ParseProcesses(debugger))
+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());
+}
+
+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 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;
 
   return llvm::Error::success();
 }

lldb/test/API/commands/trace/TestTraceLoad.py

def testLoadTrace(self):
         self.assertEqual(module.GetSectionAtIndex(0).GetFileAddress(), 0x400000)
 
         self.assertEqual("6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A", module.GetUUIDString())
 
 
     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"])
-
-        # 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"])
+        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]
+
+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)

lldb/test/API/commands/trace/intelpt-trace/trace_bad4.json

+{
+  "trace": {
+    "type": "intel-pt",
+    "pt_cpu": {
+      "vendor": "intel",
+      "model": 79,
+      "stepping": 1
+    }
+  },
+  "processes": [
+  ]
+}

lldb/test/API/commands/trace/intelpt-trace/trace_bad5.json

+{
+  "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"
+        }
+      ]
+    },
+    {}
+  ]
+}