[PATCH] Add DWARFFormValue::getAsSignedConstant().

Frederic Riss friss at apple.com
Mon Oct 6 13:23:04 PDT 2014


================
Comment at: lib/DebugInfo/DWARFFormValue.cpp:554
@@ +553,3 @@
+  if ((!isFormClass(FC_Constant) && !isFormClass(FC_Flag))
+      || Form == DW_FORM_udata)
+    return None;
----------------
dblaikie wrote:
> hmm - why is udata considered "not signed". It's specifically saying that the value is positive, so should be returned successfully that way, no?
> 
> Arguable, I suppose - but I wouldn't put it past implementations to return positive data in _udata to reduce the number of bytes needed to encode it.
Right. This check wasn't there at first and I added it to prevent issues with values > LONG_MAX being returned as negative numbers. Given how we'll use the method this isn't really an issue.

Note that getAsUnsignedConstant rejects FORM_sdata. It has more reasons to do so, but along the same line of thought, it should maybe return a value if he sdata is in fact positive? 

================
Comment at: unittests/DebugInfo/DWARFFormValueTest.cpp:103
@@ +102,3 @@
+TEST(DWARFFormValue, SignedConstantForms) {
+  DWARFFormValue data1 = createFromValue(DW_FORM_data1, 0xFF);
+  DWARFFormValue data2 = createFromValue(DW_FORM_data2, 0xFFFF);
----------------
dblaikie wrote:
> Does this need tests for any other cases? (these tests would all pass with "getAsSignedConstant()" implemented as a single "return -1")
My main concern was to check that I got the sign extension right for all the FORM_data*. Some other cases can of course be added. Do you have any specific limit tests in mind that  would be interesting, or just adding a few positive and negative values to the test would do it for you?

http://reviews.llvm.org/D5630






More information about the llvm-commits mailing list