[lldb/Core] Fix incomplete type variable dereferencing crash.

[mib]

Summary

The patch fixes a crash in ValueObject::CreateChildAtIndex caused by a
null pointer dereferencing. This is a corner case that is happening when
trying to dereference a variable with an incomplete type, and this same
variable doesn't have a synthetic value to get the child ValueObject.

If this happens, lldb will now return a null pointer that will results
in an error message.

rdar://65181171

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

lldb/source/Core/ValueObject.cpp

691

Style-nit: I think few people love early returns a much as I do, but here I think the llvm-way of checking a pointer is would be preferable over a very-late-early-exit:

if (auto* synth_valobj = GetSyntheticValue()) {
  valobj =
        synth_valobj->GetChildAtIndex(synthetic_index, synthetic_array_member)
            .get();
}

Additionally, I'm not sure if this should be auto according to the LLVM coding rules.

[davide]

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete?

[jingham]

In D83327#2136814, @davide wrote:

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete?

Doing so allows one to write a synthetic child provider that provides the fields for an incomplete type. This is useful if you don't have debug info for a given type but know its layouts by some other means.

[mib]

In D83327#2136814, @davide wrote:

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete?

The code is called because some incomplete types can actually be typedefs. In this case, we use the pointee type to create the ValueObject child (in ValueObject::CreateChildAtIndex)

[JDevlieghere]

It sounds like that would be worth adding as a comment.

[davide]

In D83327#2136842, @jingham wrote:

In D83327#2136814, @davide wrote:

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete?

Doing so allows one to write a synthetic child provider that provides the fields for an incomplete type. This is useful if you don't have debug info for a given type but know its layouts by some other means.

Interesting, thanks. Do we have an example of when this triggers?

[mib]

• Addressed style comment.
• Added a comment to give more context.

[mib]

In D83327#2136863, @davide wrote:

In D83327#2136842, @jingham wrote:

In D83327#2136814, @davide wrote:

Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete?

Doing so allows one to write a synthetic child provider that provides the fields for an incomplete type. This is useful if you don't have debug info for a given type but know its layouts by some other means.

Interesting, thanks. Do we have an example of when this triggers?

This change was first introduced by D79554, with the CFDictionaryRef formatter. They reused the same logic as NSCFDictionary but needed this to generate the ValueObject's children.

[jingham]

Other than a quibble about using _sp suffix, this is fine.

lldb/source/Core/ValueObject.cpp

693

We usually use _sp suffix for shared pointers.

[mib]

• Addressed Jim's comment.

[mib]

Patch landed in 7177e63fb554 !

[labath]

Thanks for fixing this.

lldb/source/Core/ValueObject.cpp

690

Sorry for the pedantry, but the wording "LLDB will try to ..." makes me think that there is some other part of lldb which "tries to use the synthetic value", and this code is somehow responding to that behavior.
But IIUC, this *is* the code which is using the synthetic value. In that case, it would be enough to say "In case of an incomplete type, try to use the synthetic value to ...".

lldb/source/Core/ValueObject.cpp

 //===-- ValueObject.cpp ---------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/ValueObject.h"
 
 #include "lldb/Core/Address.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ValueObjectCast.h"
 #include "lldb/Core/ValueObjectChild.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Core/ValueObjectDynamicValue.h"
 #include "lldb/Core/ValueObjectMemory.h"
if (child_compiler_type) {
 
     valobj = new ValueObjectChild(
         *this, child_compiler_type, child_name, child_byte_size,
         child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
         child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
         language_flags);
   }
 
-  if (!valobj && synthetic_array_member)
-    valobj = GetSyntheticValue()
+  // In case of an incomplete type, LLDB will try to use the ValueObject's
+  // synthetic value to create the child ValueObject.
+  if (!valobj && synthetic_array_member) {
+    if (ValueObjectSP synth_valobj_sp = GetSyntheticValue()) {
+      valobj = synth_valobj_sp
                    ->GetChildAtIndex(synthetic_index, synthetic_array_member)
                    .get();
+    }
+  }
 
   return valobj;
 }
 
 bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
                                       std::string &destination,
                                       lldb::LanguageType lang) {
   return GetSummaryAsCString(summary_ptr, destination,

lldb/test/API/functionalities/target_var/Makefile

-include Makefile.rules
+C_SOURCES := main.c
 
-a.out: globals.ll
-  $(CC) $(CFLAGS) -g -c $^ -o globals.o
-  $(LD) $(LDFLAGS) -g globals.o -o $@
+include Makefile.rules

lldb/test/API/functionalities/target_var/TestTargetVar.py

class targetCommandTestCase(TestBase):
     @skipUnlessDarwin
     @skipIfDarwinEmbedded           # needs x86_64
     @skipIf(debug_info="gmodules")  # not relevant
     @skipIf(compiler="clang", compiler_version=['<', '7.0'])
     def testTargetVarExpr(self):
         self.build()
         lldbutil.run_to_name_breakpoint(self, 'main')
         self.expect("target variable i", substrs=['i', '42'])
+        self.expect("target variable var", patterns=['\(incomplete \*\) var = 0[xX](0)*dead'])
+        self.expect("target variable var[0]", error=True, substrs=["can't find global variable 'var[0]'"])

lldb/test/API/functionalities/target_var/globals.c

-int i = 42;
-int *p = &i;
-
-int main() {
-  return *p;
-}

lldb/test/API/functionalities/target_var/globals.ll

-source_filename = "globals.c"
-target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-apple-macosx10.14.0"
-
-@i = global i32 42, align 4
-@p = global i32* @i, align 8, !dbg !0, !dbg !6
-
-; Function Attrs: noinline nounwind optnone ssp uwtable
-define i32 @main() #0 !dbg !15 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval, align 4
-  %0 = load i32*, i32** @p, align 8, !dbg !18
-  %1 = load i32, i32* %0, align 4, !dbg !18
-  ret i32 %1, !dbg !18
-}
-
-attributes #0 = { noinline nounwind optnone ssp uwtable }
-
-!llvm.dbg.cu = !{!2}
-!llvm.module.flags = !{!10, !11, !12, !13}
-!llvm.ident = !{!14}
-
-!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression(DW_OP_deref))
-!1 = distinct !DIGlobalVariable(name: "i", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
-!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, emissionKind: FullDebug, globals: !5)
-!3 = !DIFile(filename: "globals.c", directory: "/")
-!4 = !{}
-!5 = !{!0, !6}
-!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
-!7 = distinct !DIGlobalVariable(name: "p", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
-!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
-!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!10 = !{i32 2, !"Dwarf Version", i32 4}
-!11 = !{i32 2, !"Debug Info Version", i32 3}
-!12 = !{i32 1, !"wchar_size", i32 4}
-!13 = !{i32 7, !"PIC Level", i32 2}
-!14 = !{!"clang version 8.0.0 (trunk 340838) (llvm/trunk 340843)"}
-!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: false, unit: !2, retainedNodes: !4)
-!16 = !DISubroutineType(types: !17)
-!17 = !{!9}
-!18 = !DILocation(line: 5, scope: !15)

lldb/test/API/functionalities/target_var/main.c

+int i = 42;
+int *p = &i;
+
+struct incomplete;
+struct incomplete *var = (struct incomplete *)0xdead;
+
+int main() { return *p; }