[dwarfdump] Prettyprint DW_AT_APPLE_property_attribute bitfield values.

[friss]

Summary

This change depends on the ApplePropertyString helper that I sent spearately.
Not sure how you want this tested: as a tool test by adding a binary to dump, or as an llvm test starting from an IR file?

[dblaikie]

Looks reasonable (apart from testing) - depends how you want to do it. I assume there are some existing tests that test properties that could be updated to use this feature? (does this not regress any existing tests?)

If there aren't any such tests, I imagine we want some - and they could just test this incidentally.

(I'm usually on the fence about writing tool tests separately from unit tests... *shrug* sometimes it's nice, sometimes it seems like overkill)

[friss]

In D5689#4, @dblaikie wrote:

Looks reasonable (apart from testing) - depends how you want to do it. I assume there are some existing tests that test properties that could be updated to use this feature? (does this not regress any existing tests?)

If there aren't any such tests, I imagine we want some - and they could just test this incidentally.

(I'm usually on the fence about writing tool tests separately from unit tests... *shrug* sometimes it's nice, sometimes it seems like overkill)

There aren't any. I discovered the discrepancy between the enum values yesterday by trying to produce some tests and the results didn't actually make sense. I can add an llvm test, sure, but my ObjC foo being what it is, I fear that the code won't make any sense :-) Thus it won't test much except the actual dumping.

[dblaikie]

Probably sensible enough then to just make it a dumping test - I think
that's generally 'better'/purist when adding dumper features, I just
understand it's sometimes a bit more fuss. So I'll never complain about you
doing it that way (checking a small sample source, a binary, etc - you can
see the other pure dwarfdump tests).

& since you guys are planning to make this more of a production tool, I
think it's a good idea to have more tests for dwarfdump itself - so that we
don't accidentally make holes in the dwarfdump test coverage when we
refactor tests.

[samsonov]

Yep, looks good once we test this.

lib/DebugInfo/DWARFDebugInfoEntry.cpp

87 ↗

(On Diff #14616)

This would output an extra trailing comma

[friss]

lib/DebugInfo/DWARFDebugInfoEntry.cpp

87 ↗

(On Diff #14616)

Nope because the condition just above exits if we've handled the last bit. (The actual loop exit test is in the middle of the loop. I find this idiom less clumsy than having a bool lying around telling if you are in the first iteration or not, but I can understand opinions might differ on this point).

[samsonov]

lib/DebugInfo/DWARFDebugInfoEntry.cpp

87 ↗

(On Diff #14616)

Oh, sorry, I missed that =/ The code is fine as is, I have no strong preference.

[friss]

Added a dwarfdump tool test for ObjC functionality (for now properties dumping).

[dblaikie]

test/DebugInfo/dwarfdump-objc.test

21 ↗

(On Diff #14680)

(0x0c ( DW_APPLE_PROPERTY_foo, bar ) )

seems inconsistent use of space - there's no leading space before the 0x0c, but there's a leading space before DW (& then trailing spaces for both ')'.

I'd probably be in favor of no leading or trailing spaces, but I'm not sure what this looks like compared to various other parts of the dump output.

[friss]

test/DebugInfo/dwarfdump-objc.test

21 ↗

(On Diff #14680)

Agreed, it doesn't look nice. I just copied the formatting from the native dwarfdump, but I like it better with leading and trailing spaces (there isn't any prior example of that kind of dump input I think, so just pick what seems nicer). I'll resubmit shortly.

[friss]

Removed some spaces from the dump format.

[dblaikie]

Looks good to me - please submit.

(it seems most of our existing formatting doesn't put extra spaces - though we're not entirely consistent (strp has a leading space, exprloc (& maybe other FORM_data) has a trailing space (probably just because someone didn't handle the end case), etc)

[friss]

Closed by commit rL219507 (authored by @friss).

llvm/trunk/lib/DebugInfo/DWARFDebugInfoEntry.cpp

 //===-- DWARFDebugInfoEntry.cpp -------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DWARFDebugInfoEntry.h"
 #include "DWARFCompileUnit.h"
 #include "DWARFContext.h"
 #include "DWARFDebugAbbrev.h"
 #include "llvm/DebugInfo/DWARFFormValue.h"
+#include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 using namespace dwarf;
 
 // Small helper to extract a DIE pointed by a reference
if (abbrCode) {
            << abbrCode << '\n';
       }
     } else {
       OS.indent(indent) << "NULL\n";
     }
   }
 }
 
+static void dumpApplePropertyAttribute(raw_ostream &OS, uint64_t Val) {
+  OS << " (";
+  do {
+    uint64_t Bit = 1ULL << countTrailingZeros(Val);
+    if (const char *PropName = ApplePropertyString(Bit))
+      OS << PropName;
+    else
+      OS << format("DW_APPLE_PROPERTY_0x%" PRIx64, Bit);
+    if (!(Val ^= Bit))
+      break;
+    OS << ", ";
+  } while (true);
+  OS << ")";
+}
+
 void DWARFDebugInfoEntryMinimal::dumpAttribute(raw_ostream &OS,
                                                DWARFUnit *u,
                                                uint32_t *offset_ptr,
                                                uint16_t attr, uint16_t form,
                                                unsigned indent) const {
   OS << "            ";
   OS.indent(indent+2);
   const char *attrString = AttributeString(attr);
void DWARFDebugInfoEntryMinimal::dumpAttribute(raw_ostream &OS,
   if ((attr == DW_AT_specification || attr == DW_AT_abstract_origin) &&
       // The signature references aren't handled.
       formValue.getForm() != DW_FORM_ref_sig8) {
     uint32_t Ref = formValue.getAsReference(u).getValue();
     DWARFDebugInfoEntryMinimal DIE;
     if (const DWARFUnit *RefU = findUnitAndExtractFast(DIE, u, &Ref))
       if (const char *Ref = DIE.getName(RefU, DINameKind::LinkageName))
         OS << " \"" << Ref << '\"';
+  } else if (attr == DW_AT_APPLE_property_attribute) {
+    if (Optional<uint64_t> OptVal = formValue.getAsUnsignedConstant())
+      dumpApplePropertyAttribute(OS, *OptVal);
   }
 
   OS << ")\n";
 }
 
 bool DWARFDebugInfoEntryMinimal::extractFast(const DWARFUnit *U,
                                              uint32_t *OffsetPtr) {
   Offset = *OffsetPtr;

llvm/trunk/test/DebugInfo/Inputs/dwarfdump-objc.m

+// Compile with clang -g dwarfdump-objc.m -c -Wno-objc-root-class
+
+@interface NSObject {} @end
+
+
+@interface TestInterface
+@property (readonly) int ReadOnly;
+@property (assign) int Assign;
+@property (readwrite) int ReadWrite;
+@property (retain) NSObject *Retain;
+@property (copy) NSObject *Copy;
+@property (nonatomic) int NonAtomic;
+@end
+
+@implementation TestInterface
+@end

llvm/trunk/test/DebugInfo/dwarfdump-objc.test

+RUN: llvm-dwarfdump %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s
+
+CHECK:      .debug_info contents:
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:    DW_AT_APPLE_property_name {{.*}} "ReadOnly"
+CHECK-NOT: TAG
+CHECK:    DW_AT_APPLE_property_attribute {{.*}} (0x01 (DW_APPLE_PROPERTY_readonly))
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_name {{.*}} "Assign"
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_attribute {{.*}} (0x0c (DW_APPLE_PROPERTY_assign, DW_APPLE_PROPERTY_readwrite))
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_name {{.*}} "ReadWrite"
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_attribute {{.*}} (0x0c (DW_APPLE_PROPERTY_assign, DW_APPLE_PROPERTY_readwrite))
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_name {{.*}} "Retain"
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_attribute {{.*}} (0x18 (DW_APPLE_PROPERTY_readwrite, DW_APPLE_PROPERTY_retain))
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_name {{.*}} "Copy"
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_attribute {{.*}} (0x28 (DW_APPLE_PROPERTY_readwrite, DW_APPLE_PROPERTY_copy))
+
+CHECK: DW_TAG_APPLE_property
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_name {{.*}} "NonAtomic"
+CHECK-NOT: TAG
+CHECK:   DW_AT_APPLE_property_attribute {{.*}} (0x4c (DW_APPLE_PROPERTY_assign, DW_APPLE_PROPERTY_readwrite, DW_APPLE_PROPERTY_nonatomic))
+