[llvm-commits] [PATCH] Add support for CodeView debug information

[NAKAMURA Takumi]

  Thank you for working on it!

  With my quick build, I found some issues.

  # I would like you to check also on non-windows hosts, Linux or Darwin. clang-3.2rc is better.
  # This breaks a few codegen/x86 tests on mingw and dozen of tests in llvm and clang on msvc.
  You should not break any tests, or you should update tests with clear reason.
  # Ok, you would know, **TESTCASE** please!
  I suggest you might begin to write dump utility for this. Then...
  ## Verify your new utility with well-known files, for example, cl.exe generated. (I have not tried non-pdb debug, though)
  ## (Additional) Add tests for the utility to confirm minimal requirements.
  ## Add tests for the debug infos with the utility.
  # It seems this patch would be ignorant of endianness.
  I think, for now, we could go as far as you add comments, FIXMEs or W-I-P.

  FYI, clang reports warnings like as below;


================
Comment at: include/llvm/Support/CodeView.h:417
@@ +416,3 @@
+  T_Far32_Ptr_Bits = 0x0500,
+  T_Near64_Ptr_Bits = 0x0600,
+};
----------------
warning: commas at the end of enumerator lists are a C++11 extension [-Wc++11-extensions]

================
Comment at: include/llvm/Support/CodeView.h:463
@@ +462,3 @@
+  CV_CALL_PPCCALL     = 0x0f, // PPC call
+  CV_CALL_RESERVED    = 0x10, // first unused call enumeration
+
----------------
warning: commas at the end of enumerator lists are a C++11 extension [-Wc++11-extensions]

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.h:66
@@ +65,3 @@
+public:
+  TypeStringManager() : NextIndex(0), TypesString(LeafVector()) {}
+
----------------
warning: field 'NextIndex' will be initialized after field 'TypesString' [-Wreorder]

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:169
@@ +168,3 @@
+    RV = CodeView::T_Real << CodeView::T_Type_Shift,
+    RiV = CodeView::T_Real_int_value << CodeView::T_Type_Shift,
+
----------------
warning: commas at the end of enumerator lists are a C++11 extension [-Wc++11-extensions]

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:550
@@ +549,1 @@
+}
\ No newline at end of file

----------------
Please fix it in next time ;)

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:132
@@ +131,3 @@
+  for (LeafVector::const_iterator I = TypesString.cbegin(),
+                                  E =  TypesString.cend();
+                                  I != E;
----------------
Don't use cbegin() and cend. LLVM doesn't require C++11.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:145
@@ +144,3 @@
+  for (LeafVector::const_iterator I = TypesString.cbegin(),
+                                  E =  TypesString.cend();
+                                  I != E;
----------------
Ditto.

================
Comment at: include/llvm/Support/CodeView.h:605
@@ +604,3 @@
+  Procedure_V1 Procedure_V1;
+  Procedure_V2 Procedure_V2;
+};
----------------
5 of types clashes against each member. g++-4.4, I am using, cannot accept them.

# Sink them into a namespace.
# Mangle each of them, for example, Arglist_V2_t.

================
Comment at: include/llvm/Support/CodeView.h:569
@@ +568,3 @@
+  uint16_t leaf;           // LF_FIELDLIST
+  char data[];             // field list sub lists
+};
----------------
(g++-4.4 -pedantic) warning: ISO C++ forbids zero-size array ‘data’

================
Comment at: include/llvm/Support/CodeView.h:578
@@ +577,3 @@
+  CV_fldattr_t    attr;       // attribute mask
+  unsigned char   offset[];   // variable length offset of field followed
+                              // by length prefixed name of field
----------------
(g++-4.4 -pedantic) warning: ISO C++ forbids zero-size array ‘offset’


http://llvm-reviews.chandlerc.com/D165