Adding a DIBuilder interface for Fortran's assumed length string

[ykhatav]

Summary

Currently DIBuilder supports constant length strings only. In this review I have added a new interface called createStringType() which is capable of generating the debug info metadata for an assumed length string.

[cchen15]

I just added a new field StringLocationExp to DIStringType in this commit: https://reviews.llvm.org/rG28bfa57a7315c3161124c90b4d52f467616dc92e . Please add an optional argument to these createStringType functions so that the caller can specify this new field if it desires.

llvm/include/llvm/IR/DIBuilder.h

228

stringLength can be typed as DIVariable *.

Side note: LLVM coding convention calls for capital first letter of a variable name; so stringLength -> StringLength.

[ykhatav]

Added StringLocationExp to the new apis

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

222–240

Use DIVariable * for StringLength.

223

Use DIExpression * for StrLocationExp.

223

Ditto.

llvm/unittests/IR/DebugInfoTest.cpp

249–289

Using nullptr for the length argument seems too generic. Could you mock up a DIVairbale/DIExpression for length and a DIExpression for location for both new functions?

[ykhatav]

Updated the test to include the proper string functions

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

223–241

Now the types of the two new functions and the original are all different, you can overload the name createStringType here.

[ykhatav]

Worked on review comments

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

223–241

One last suggestion: We can combine the two new functions into one by typing the length argument PointerUnion<DIVariable*, DIExpression*>. Please see DIBuilder::createArrayType for a precedent.

[bwyma]

llvm/include/llvm/IR/DIBuilder.h

223–241

In the createArrayType() instance, the expression and variable arguments are interchangeable metadata to the same composite type parameter. In DIStringType, at least currently, the variable string length and string length expression are independent parameters. To reflect this in the DIBuilder interface I would prefer to have independent createStringType() calls.

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

223–241

DIStringType should have only one field to represent the string length. I think this boils down to if the DIBuilder interface needs to reflect the current state of the underlying implementation. My take is that we can go ahead with what the user should see with this patch, and then change the implementation at a later time if necessary to avoid an additional API change down the road.

[bwyma]

llvm/include/llvm/IR/DIBuilder.h

223–241

Even if this change was made, there will still be a second interface routine for creating constant string length types. I don't see value in upcasting the arguments and performing isa comparisons on them just to downcast them into separate arguments again. The string type does not yet have the complexity of the array type interface. The proposed implementation here is cleaner and functionally equivalent.

[cchen15]

llvm/include/llvm/IR/DIBuilder.h

223–241

I don't fully agree but I will commit.

Please wait for the final approval from @aprantl.

[ykhatav]

Updated my local branch with the latest changes

[aprantl]

Can you add documentation for how these String types are supposed to be used to SourceLevelDebugging.rst?
What's an example for a memory location expression that flang would produce?

[ykhatav]

Modified SourceLevelDebugging.rst to include information about memory location exp.

[aprantl]

llvm/docs/SourceLevelDebugging.rst

1092

Thanks. It was unclear to me what to expect in the expression. If it's using DW_OP_push_object_address that makes sense.

[ykhatav]

Commit id: 70fdbf35de1c37a22a5321ed7d7d2c190d788549

llvm/docs/SourceLevelDebugging.rst

 
 a Fortran front-end would generate the following descriptors:
 
 .. code-block:: text
 
   !DILocalVariable(name: "string", arg: 1, scope: !10, file: !3, line: 4, type: !15)
   !DIStringType(name: "character(*)!2", stringLength: !16, stringLengthExpression: !DIExpression(), size: 32)
 
+A fortran deferred-length character can also contain the information of raw storage of the characters in addition to the length of the string. This information is encoded in the  stringLocationExpression field. Based on this information, DW_AT_data_location attribute is emitted in a DW_TAG_string_type debug info. 
+
+  !DIStringType(name: "character(*)!2", stringLengthExpression: !DIExpression(), stringLocationExpression: !DIExpression(DW_OP_push_object_address, DW_OP_deref), size: 32)
+
 and this will materialize in DWARF tags as:
 
 .. code-block:: text
 
    DW_TAG_string_type
                 DW_AT_name      ("character(*)!2")
                 DW_AT_string_length     (0x00000064)
    0x00000064:    DW_TAG_variable
                   DW_AT_location      (DW_OP_fbreg +16)
                   DW_AT_type  (0x00000083 "integer*8")
+                  DW_AT_data_location (DW_OP_push_object_address, DW_OP_deref)
                   ...
                   DW_AT_artificial    (true)
 
 Debugging information format
 ============================
 
 Debugging Information Extension for Objective C Properties
 ----------------------------------------------------------

llvm/include/llvm/IR/DIBuilder.h

public:
     DIBasicType *createBasicType(StringRef Name, uint64_t SizeInBits,
                                  unsigned Encoding,
                                  DINode::DIFlags Flags = DINode::FlagZero);
 
     /// Create debugging information entry for a string
     /// type.
     /// \param Name        Type name.
     /// \param SizeInBits  Size of the type.
     DIStringType *createStringType(StringRef Name, uint64_t SizeInBits);
 
+    /// Create debugging information entry for Fortran
+    /// assumed length string type.
+    /// \param Name            Type name.
+    /// \param StringLength    String length expressed as DIVariable *.
+    /// \param StrLocationExp  Optional memory location of the string.
+    DIStringType *createStringType(StringRef Name, DIVariable *StringLength,
+                                   DIExpression *StrLocationExp = nullptr);
+
+    /// Create debugging information entry for Fortran
+    /// assumed length string type.
+    /// \param Name             Type name.
+    /// \param StringLengthExp  String length expressed in DIExpression form.
+    /// \param StrLocationExp   Optional memory location of the string.
+    DIStringType *createStringType(StringRef Name,
+                                   DIExpression *StringLengthExp,
+                                   DIExpression *StrLocationExp = nullptr);
+
     /// Create debugging information entry for a qualified
     /// type, e.g. 'const int'.
     /// \param Tag         Tag identifing type, e.g. dwarf::TAG_volatile_type
     /// \param FromTy      Base Type.
     DIDerivedType *createQualifiedType(unsigned Tag, DIType *FromTy);
 
     /// Create debugging information entry for a pointer.
     /// \param PointeeTy         Type pointed by this pointer.
     /// \param SizeInBits        Size.

llvm/lib/IR/DIBuilder.cpp

 }
 
 DIStringType *DIBuilder::createStringType(StringRef Name, uint64_t SizeInBits) {
   assert(!Name.empty() && "Unable to create type without name");
   return DIStringType::get(VMContext, dwarf::DW_TAG_string_type, Name,
                            SizeInBits, 0);
 }
 
+DIStringType *DIBuilder::createStringType(StringRef Name,
+                                          DIVariable *StringLength,
+                                          DIExpression *StrLocationExp) {
+  assert(!Name.empty() && "Unable to create type without name");
+  return DIStringType::get(VMContext, dwarf::DW_TAG_string_type, Name,
+                           StringLength, nullptr, StrLocationExp, 0, 0, 0);
+}
+
+DIStringType *DIBuilder::createStringType(StringRef Name,
+                                          DIExpression *StringLengthExp,
+                                          DIExpression *StrLocationExp) {
+  assert(!Name.empty() && "Unable to create type without name");
+  return DIStringType::get(VMContext, dwarf::DW_TAG_string_type, Name, nullptr,
+                           StringLengthExp, StrLocationExp, 0, 0, 0);
+}
+
 DIDerivedType *DIBuilder::createQualifiedType(unsigned Tag, DIType *FromTy) {
   return DIDerivedType::get(VMContext, Tag, "", nullptr, 0, nullptr, FromTy, 0,
                             0, 0, None, DINode::FlagZero);
 }
 
 DIDerivedType *
 DIBuilder::createPointerType(DIType *PointeeTy, uint64_t SizeInBits,
                              uint32_t AlignInBits,

llvm/unittests/IR/DebugInfoTest.cpp

DIScope *Scope = DISubprogram::getDistinct(
       Ctx, nullptr, "", "", nullptr, 0, nullptr, 0, nullptr, 0, 0,
       DINode::FlagZero, DISubprogram::SPFlagZero, nullptr);
   DIType *Type = DIB.createBasicType("Int", 64, dwarf::DW_ATE_signed);
   DIFile *F = DIB.createFile("main.c", "/");
 
   DIDerivedType *SetType = DIB.createSetType(Scope, "set1", F, 1, 64, 64, Type);
   EXPECT_TRUE(isa_and_nonnull<DIDerivedType>(SetType));
 }
 
+TEST(DIBuilder, CreateStringType) {
+  LLVMContext Ctx;
+  std::unique_ptr<Module> M(new Module("MyModule", Ctx));
+  DIBuilder DIB(*M);
+  DIScope *Scope = DISubprogram::getDistinct(
+      Ctx, nullptr, "", "", nullptr, 0, nullptr, 0, nullptr, 0, 0,
+      DINode::FlagZero, DISubprogram::SPFlagZero, nullptr);
+  DIFile *F = DIB.createFile("main.c", "/");
+  StringRef StrName = "string";
+  DIVariable *StringLen = DIB.createAutoVariable(Scope, StrName, F, 0, nullptr,
+                                                 false, DINode::FlagZero, 0);
+  auto getDIExpression = [&DIB](int offset) {
+    SmallVector<uint64_t, 4> ops;
+    ops.push_back(llvm::dwarf::DW_OP_push_object_address);
+    DIExpression::appendOffset(ops, offset);
+    ops.push_back(llvm::dwarf::DW_OP_deref);
+
+    return DIB.createExpression(ops);
+  };
+  DIExpression *StringLocationExp = getDIExpression(1);
+  DIStringType *StringType =
+      DIB.createStringType(StrName, StringLen, StringLocationExp);
+
+  EXPECT_TRUE(isa_and_nonnull<DIStringType>(StringType));
+  EXPECT_EQ(StringType->getName(), StrName);
+  EXPECT_EQ(StringType->getStringLength(), StringLen);
+  EXPECT_EQ(StringType->getStringLocationExp(), StringLocationExp);
+
+  StringRef StrNameExp = "stringexp";
+  DIExpression *StringLengthExp = getDIExpression(2);
+  DIStringType *StringTypeExp =
+      DIB.createStringType(StrNameExp, StringLengthExp, StringLocationExp);
+
+  EXPECT_TRUE(isa_and_nonnull<DIStringType>(StringTypeExp));
+  EXPECT_EQ(StringTypeExp->getName(), StrNameExp);
+  EXPECT_EQ(StringTypeExp->getStringLocationExp(), StringLocationExp);
+  EXPECT_EQ(StringTypeExp->getStringLengthExp(), StringLengthExp);
+}
+
 TEST(DIBuilder, DIEnumerator) {
   LLVMContext Ctx;
   std::unique_ptr<Module> M(new Module("MyModule", Ctx));
   DIBuilder DIB(*M);
   APSInt I1(APInt(32, 1));
   APSInt I2(APInt(33, 1));
 
   auto *E = DIEnumerator::get(Ctx, I1, I1.isSigned(), "name");
   EXPECT_TRUE(E);