[lldb] Summary provider for char flexible array members

[labath]

Summary

Add a summary provider which can print char[] members at the ends of
structs.

Depends on D112709.

[jingham]

Regex Type summary matching happens after the ConstString matching. Enrico did it that way because the ConstString matching is so much quicker, so if we can find a match there we'll get to it more quickly...

So this patch moves the char * recognition from the beginning of the type summary matching to the end, and potentially makes it a slower match.

I doubt that this will be noticeable on modern systems, however, just something to keep in mind.

It also changes the order of search slightly. I think this is observable: it would mean a regex that happens to match "char *" as well as other things used to not be chosen for "char *" because it would have hit the ConstString summary first. Now it will match, because the built-in regex summary will be checked after the user added ones.

Again, I don't think this is a reason not to do the patch. But something to keep in mind. The code itself looks fine.

[shafik]

In D113174#3109664, @jingham wrote:

Regex Type summary matching happens after the ConstString matching. Enrico did it that way because the ConstString matching is so much quicker, so if we can find a match there we'll get to it more quickly...

So this patch moves the char * recognition from the beginning of the type summary matching to the end, and potentially makes it a slower match.

I doubt that this will be noticeable on modern systems, however, just something to keep in mind.

It also changes the order of search slightly. I think this is observable: it would mean a regex that happens to match "char *" as well as other things used to not be chosen for "char *" because it would have hit the ConstString summary first. Now it will match, because the built-in regex summary will be checked after the user added ones.

Again, I don't think this is a reason not to do the patch. But something to keep in mind. The code itself looks fine.

I was looking at these char formatters a while ago and IIRC I did see some impact making these regex matches but I don't remember the details and did not look closely (so maybe there was another effect at play) b/c I went with a different approach.

[labath]

In D113174#3112464, @shafik wrote:

In D113174#3109664, @jingham wrote:

Regex Type summary matching happens after the ConstString matching. Enrico did it that way because the ConstString matching is so much quicker, so if we can find a match there we'll get to it more quickly...

So this patch moves the char * recognition from the beginning of the type summary matching to the end, and potentially makes it a slower match.

I doubt that this will be noticeable on modern systems, however, just something to keep in mind.

It also changes the order of search slightly. I think this is observable: it would mean a regex that happens to match "char *" as well as other things used to not be chosen for "char *" because it would have hit the ConstString summary first. Now it will match, because the built-in regex summary will be checked after the user added ones.

Again, I don't think this is a reason not to do the patch. But something to keep in mind. The code itself looks fine.

I was looking at these char formatters a while ago and IIRC I did see some impact making these regex matches but I don't remember the details and did not look closely (so maybe there was another effect at play) b/c I went with a different approach.

This is still a bounded set, so it is possible to replace this regex with 6 strings. I did a regex because it makes the code shorter, but I have no problem with spelling this out.

lldb/source/DataFormatters/FormatManager.cpp

void FormatManager::LoadSystemFormatters() {
   lldb::TypeSummaryImplSP string_array_format(
       new StringSummaryFormat(string_array_flags, "${var%char[]}"));
 
   RegularExpression any_size_char_arr(R"(^((un)?signed )?char ?\[[0-9]+\]$)");
 
   TypeCategoryImpl::SharedPointer sys_category_sp =
       GetCategory(m_system_category_name);
 
-  sys_category_sp->GetTypeSummariesContainer()->Add(ConstString("char *"),
-                                                    string_format);
-  sys_category_sp->GetTypeSummariesContainer()->Add(
-      ConstString("unsigned char *"), string_format);
-  sys_category_sp->GetTypeSummariesContainer()->Add(
-      ConstString("signed char *"), string_format);
+  sys_category_sp->GetRegexTypeSummariesContainer()->Add(
+      RegularExpression(R"(^((un)?signed )?char ?(\*|\[\])$)"), string_format);
 
   sys_category_sp->GetRegexTypeSummariesContainer()->Add(
       std::move(any_size_char_arr), string_array_format);
 
   lldb::TypeSummaryImplSP ostype_summary(
       new StringSummaryFormat(TypeSummaryImpl::Flags()
                                   .SetCascades(false)
                                   .SetSkipPointers(true)

lldb/test/API/functionalities/data-formatter/type_summary_list_arg/TestTypeSummaryListArg.py

def test_type_summary_list_with_arg(self):
         self.expect(
             'type summary list Foo',
             substrs=[
                 'Category: default',
                 'Category: system'])
         self.expect(
             'type summary list char',
             substrs=[
-                'char *',
-                'unsigned char'])
+                'char ?(\*|\[\])',
+                'char ?\[[0-9]+\]'])
 
         self.expect(
             'type summary list -w default',
             substrs=['system'],
             matching=False)
         self.expect(
             'type summary list -w system unsigned',
             substrs=[
                 'default',
                 '0-9'],
             matching=False)
         self.expect(
             'type summary list -w system char',
-            substrs=['unsigned char *'],
+            substrs=[
+                'char ?(\*|\[\])',
+                'char ?\[[0-9]+\]'],
             matching=True)

lldb/test/API/lang/c/flexible-array-members/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/lang/c/flexible-array-members/TestCFlexibleArrayMembers.py

+"""
+Tests C99's flexible array members.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "// break here",
+                lldb.SBFileSpec("main.c"))
+
+        self.expect_var_path("c->flexible", type="char[]", summary='"contents"')
+        self.expect_var_path("sc->flexible", type="signed char[]", summary='"contents"')
+        self.expect_var_path("uc->flexible", type="unsigned char[]", summary='"contents"')
+        # TODO: Make this work
+        self.expect("expr c->flexible", error=True,
+                substrs=["incomplete", "char[]"])
+        self.expect("expr sc->flexible", error=True,
+                substrs=["incomplete", "signed char[]"])
+        self.expect("expr uc->flexible", error=True,
+                substrs=["incomplete", "unsigned char[]"])

lldb/test/API/lang/c/flexible-array-members/main.c

+#include <stdlib.h>
+#include <string.h>
+
+struct WithFlexChar {
+  int member;
+  char flexible[];
+};
+
+struct WithFlexSChar {
+  int member;
+  signed char flexible[];
+};
+
+struct WithFlexUChar {
+  int member;
+  unsigned char flexible[];
+};
+
+#define CONTENTS "contents"
+
+int main() {
+  struct WithFlexChar *c =
+      (struct WithFlexChar *)malloc(sizeof(int) + sizeof(CONTENTS));
+  c->member = 1;
+  strcpy(c->flexible, CONTENTS);
+
+  struct WithFlexSChar *sc =
+      (struct WithFlexSChar *)malloc(sizeof(int) + sizeof(CONTENTS));
+  sc->member = 1;
+  strcpy((char *)sc->flexible, CONTENTS);
+
+  struct WithFlexUChar *uc =
+      (struct WithFlexUChar *)malloc(sizeof(int) + sizeof(CONTENTS));
+  uc->member = 1;
+  strcpy((char *)uc->flexible, CONTENTS);
+  return 0; // break here
+}