fjricci created this revision.
fjricci added reviewers: jingham, clayborg, jasonmolenda.
fjricci added subscribers: sas, lldb-commits.

If the remote uses include features when communicating
xml register info back to lldb, the existing code would reset the 
lldb register index at the beginning of each include node.
This would lead to multiple registers having the same lldb register index.
Since the lldb register numbers should be contiguous and unique,
maintain them accross the parsing of all of the xml feature nodes.

http://reviews.llvm.org/D19303

Files:
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4372,14 +4372,11 @@
 };
 
 bool
-ParseRegisters (XMLNode feature_node, GdbServerTargetInfo &target_info, 
GDBRemoteDynamicRegisterInfo &dyn_reg_info, ABISP abi_sp)
+ParseRegisters (XMLNode feature_node, GdbServerTargetInfo &target_info, 
GDBRemoteDynamicRegisterInfo &dyn_reg_info, ABISP abi_sp, uint32_t 
&cur_reg_num, uint32_t &reg_offset)
 {
     if (!feature_node)
         return false;
 
-    uint32_t cur_reg_num = 0;
-    uint32_t reg_offset = 0;
-
     feature_node.ForEachChildElementWithName("reg", [&target_info, 
&dyn_reg_info, &cur_reg_num, &reg_offset, &abi_sp](const XMLNode &reg_node) -> 
bool {
         std::string gdb_group;
         std::string gdb_type;
@@ -4635,12 +4632,16 @@
                 return true; // Keep iterating through all children of the 
target_node
             });
 
+            // Initialize these outside of ParseRegisters, since they should 
not be reset inside each include feature
+            uint32_t cur_reg_num = 0;
+            uint32_t reg_offset = 0;
+
             // Don't use Process::GetABI, this code gets called from 
DidAttach, and in that context we haven't
             // set the Target's architecture yet, so the ABI is also 
potentially incorrect.
             ABISP abi_to_use_sp = ABI::FindPlugin(arch_to_use);
             if (feature_node)
             {
-                ParseRegisters(feature_node, target_info, 
this->m_register_info, abi_to_use_sp);
+                ParseRegisters(feature_node, target_info, 
this->m_register_info, abi_to_use_sp, cur_reg_num, reg_offset);
             }
 
             for (const auto &include : target_info.includes)
@@ -4658,7 +4659,7 @@
                 XMLNode include_feature_node = 
include_xml_document.GetRootElement("feature");
                 if (include_feature_node)
                 {
-                    ParseRegisters(include_feature_node, target_info, 
this->m_register_info, abi_to_use_sp);
+                    ParseRegisters(include_feature_node, target_info, 
this->m_register_info, abi_to_use_sp, cur_reg_num, reg_offset);
                 }
             }
             this->m_register_info.Finalize(arch_to_use);


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4372,14 +4372,11 @@
 };
 
 bool
-ParseRegisters (XMLNode feature_node, GdbServerTargetInfo &target_info, GDBRemoteDynamicRegisterInfo &dyn_reg_info, ABISP abi_sp)
+ParseRegisters (XMLNode feature_node, GdbServerTargetInfo &target_info, GDBRemoteDynamicRegisterInfo &dyn_reg_info, ABISP abi_sp, uint32_t &cur_reg_num, uint32_t &reg_offset)
 {
     if (!feature_node)
         return false;
 
-    uint32_t cur_reg_num = 0;
-    uint32_t reg_offset = 0;
-
     feature_node.ForEachChildElementWithName("reg", [&target_info, &dyn_reg_info, &cur_reg_num, &reg_offset, &abi_sp](const XMLNode &reg_node) -> bool {
         std::string gdb_group;
         std::string gdb_type;
@@ -4635,12 +4632,16 @@
                 return true; // Keep iterating through all children of the target_node
             });
 
+            // Initialize these outside of ParseRegisters, since they should not be reset inside each include feature
+            uint32_t cur_reg_num = 0;
+            uint32_t reg_offset = 0;
+
             // Don't use Process::GetABI, this code gets called from DidAttach, and in that context we haven't
             // set the Target's architecture yet, so the ABI is also potentially incorrect.
             ABISP abi_to_use_sp = ABI::FindPlugin(arch_to_use);
             if (feature_node)
             {
-                ParseRegisters(feature_node, target_info, this->m_register_info, abi_to_use_sp);
+                ParseRegisters(feature_node, target_info, this->m_register_info, abi_to_use_sp, cur_reg_num, reg_offset);
             }
 
             for (const auto &include : target_info.includes)
@@ -4658,7 +4659,7 @@
                 XMLNode include_feature_node = include_xml_document.GetRootElement("feature");
                 if (include_feature_node)
                 {
-                    ParseRegisters(include_feature_node, target_info, this->m_register_info, abi_to_use_sp);
+                    ParseRegisters(include_feature_node, target_info, this->m_register_info, abi_to_use_sp, cur_reg_num, reg_offset);
                 }
             }
             this->m_register_info.Finalize(arch_to_use);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to