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

[DavidSpickett]

Summary

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.

[jasonmolenda]

LGTM.

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

feature_node.ForEachChildElementWithName(
           }
           return true; // Keep iterating through all attributes
         });
 
         if (!gdb_type.empty()) {
           // 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
           // does no harm).
           if (!gdb_type.empty() && !(encoding_set || format_set)) {
             if (llvm::StringRef(gdb_type).startswith("int")) {
               reg_info.format = eFormatHex;
               reg_info.encoding = eEncodingUint;

lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py

def test_flags_requried_attributes(self):
           </flags>
           <reg name="pc" bitsize="64"/>
           <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
 
         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):
         # lldb also sends "format". If that is set, we should still read the
         # flags type.
         self.setup_register_test("""\
           <flags id="cpsr_flags" size="4">
             <field name="B" start="0" end="0"/>
           </flags>
           <reg name="pc" bitsize="64"/>
def test_xml_includes_multiple(self):
                <target version="1.0">
                  <architecture>aarch64</architecture>
                  <xi:include href="core.xml"/>
                  <xi:include href="core-2.xml"/>
                </target>"""),
             '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"/>
                   <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
                 </feature>"""),
             'core-2.xml' : dedent("""\
                 <?xml version="1.0"?>
                 <feature name="org.gnu.gdb.aarch64.core">
def test_xml_includes_flags_redefined(self):
                  <xi:include href="core-2.xml"/>
                </target>"""),
             # Treating xi:include as a textual include, my_flags is first defined
             # in core.xml. The second definition in core-2.xml
             # is ignored.
             '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)"])