[LLDB] Discard register flags where the size doesn't match the register

In the particular case I was looking at I autogenerated a 128 bit set
of flags that is only 64 bit. This doesn't crash lldb but it was certainly
not expected.

I suspect that we would have crashed if the top 64 bits weren't
marked as unused (or at least invoked some very undefined behaviour).

When this happens, log the details and ignore the flags. Like this:
```
Size of register flags TTBR0_EL1_flags (16 bytes) for register TTBR0_EL1 does not match the register size (8 bytes). Ignoring this set of flags.
```

Turns out a few of the tests relied on this bug so I have updated
them and added a specific test for this case.

Reviewed By: jasonmolenda

Differential Revision: https://reviews.llvm.org/D148715

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e468c82382d1b0607b4f35ce8aaf3d874eb66c8a..087344198e2266899c843d22e6ffb91bb245adf3 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4341,8 +4341,19 @@ bool ParseRegisters(
           // gdb_type could reference some flags type defined in XML.
           llvm::StringMap<std::unique_ptr<RegisterFlags>>::iterator it =
               registers_flags_types.find(gdb_type);
-          if (it != registers_flags_types.end())
-            reg_info.flags_type = it->second.get();
+          if (it != registers_flags_types.end()) {
+            auto flags_type = it->second.get();
+            if (reg_info.byte_size == flags_type->GetSize())
+              reg_info.flags_type = flags_type;
+            else
+              LLDB_LOGF(log,
+                        "ProcessGDBRemote::ParseRegisters Size of register "
+                        "flags %s (%d bytes) for "
+                        "register %s does not match the register size (%d "
+                        "bytes). Ignoring this set of flags.",
+                        flags_type->GetID().c_str(), flags_type->GetSize(),
+                        reg_info.name.AsCString(), reg_info.byte_size);
+          }
 
           // There's a slim chance that the gdb_type name is both a flags type
           // and a simple type. Just in case, look for that too (setting both

diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
index 2c930d6773c5dda49d9de72eb4fd3b29b39bc35c..3ccf3ccae6e9b7cb89107a2ae0b43fdc34e36b4d 100644 (file)
--- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -319,6 +319,20 @@ class TestXMLRegisterFlags(GDBRemoteTestBase):
 
         self.expect("register read cpsr", substrs=["(C = 1)"])
 
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_register_size_mismatch(self):
+        # If the size of the flag set found does not match the size of the
+        # register, we discard the flags.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="8">
+            <field name="C" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        self.expect("register read cpsr", substrs=["(C = 1)"], matching=False)
+
     @skipIfXmlSupportMissing
     @skipIfRemote
     def test_flags_set_even_if_format_set(self):
@@ -439,7 +453,7 @@ class TestXMLRegisterFlags(GDBRemoteTestBase):
             'core.xml' : dedent("""\
                 <?xml version="1.0"?>
                 <feature name="org.gnu.gdb.aarch64.core">
-                  <flags id="x0_flags" size="4">
+                  <flags id="x0_flags" size="8">
                     <field name="B" start="0" end="0"/>
                   </flags>
                   <reg name="pc" bitsize="64"/>
@@ -475,23 +489,23 @@ class TestXMLRegisterFlags(GDBRemoteTestBase):
             'core.xml' : dedent("""\
                 <?xml version="1.0"?>
                 <feature name="org.gnu.gdb.aarch64.core">
-                  <flags id="my_flags" size="4">
+                  <flags id="my_flags" size="8">
                     <field name="correct" start="0" end="0"/>
                   </flags>
                   <reg name="pc" bitsize="64"/>
                   <reg name="x0" regnum="0" bitsize="64" type="my_flags"/>
                 </feature>"""),
-            # The my_flags here is ignored, so cpsr will use the my_flags from above.
+            # The my_flags here is ignored, so x1 will use the my_flags from above.
             'core-2.xml' : dedent("""\
                 <?xml version="1.0"?>
                 <feature name="org.gnu.gdb.aarch64.core">
-                  <flags id="my_flags" size="4">
+                  <flags id="my_flags" size="8">
                     <field name="incorrect" start="0" end="0"/>
                   </flags>
-                  <reg name="cpsr" regnum="33" bitsize="32" type="my_flags"/>
+                  <reg name="x1" regnum="33" bitsize="64" type="my_flags"/>
                 </feature>
             """),
         })
 
         self.expect("register read x0", substrs=["(correct = 1)"])
-        self.expect("register read cpsr", substrs=["(correct = 1)"])
+        self.expect("register read x1", substrs=["(correct = 1)"])