[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