[PATCH] Add parser for the stackmap section format

Pat Gavlin pagavlin at microsoft.com
Fri Jun 12 09:59:32 PDT 2015


+1 to Swaroop's suggestion to use an ostream rather than printf.


================
Comment at: include/llvm/CodeGen/StackMaps.h:48
@@ +47,3 @@
+/// section recorded in the stackmap section.  In particular, used to record
+/// the start of each function and it's stack size.
+struct StackMapSizeRecord {
----------------
s/it's/its

================
Comment at: lib/CodeGen/StackMaps.cpp:43
@@ +42,3 @@
+  assert(offset < len);
+  Primitive rval = *(Primitive*)(data + offset);
+  offset += sizeof(rval);
----------------
sanjoy wrote:
> So the machine generating the stackmap section and the machine parsing them should have the same endianness -- should this be documented explicitly?
It would certainly be nice to have his were documented if it is intentional.

================
Comment at: lib/CodeGen/StackMaps.cpp:52
@@ +51,3 @@
+  assert(offset + sizeof(LocationRecord) <= len);
+  memcpy(this, data + offset, sizeof(LocationRecord));
+  offset += sizeof(LocationRecord);
----------------
I don't see anything that guarantees that the layout of a `LocationRecord` value matches the layout of the fields in the input buffer w.r.t. padding. I think that either `LocationRecord` needs to have `__attribute__((packed))` applied (or something similar that will work across MSVC, GCC, and clang) or each field will need to be set individually using the result of an appropriate call to `parse_primitive`.

================
Comment at: lib/CodeGen/StackMaps.cpp:149
@@ +148,3 @@
+StackMapRecord& StackMapSection::findRecordForRelPC(uint32_t RelPC) {
+  // brute force search for the moment, could be improved
+  for (unsigned i = 0; i < Records.size(); i++) {
----------------
Maybe mark these as TODOs to help them stand out a bit better.

http://reviews.llvm.org/D10377

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list