[PATCH] Add DWARFFormValue::getAsSignedConstant().

David Blaikie dblaikie at gmail.com
Mon Oct 6 15:10:22 PDT 2014


================
Comment at: lib/DebugInfo/DWARFFormValue.cpp:554
@@ +553,3 @@
+  if ((!isFormClass(FC_Constant) && !isFormClass(FC_Flag))
+      || Form == DW_FORM_udata)
+    return None;
----------------
friss wrote:
> dblaikie wrote:
> > friss wrote:
> > > 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? 
> > I believe the right thing would be for the value to be printed as signed/unsigned based on udata/sdata - or based on context (form kind, etc) if it's dataN. Not sure how convenient that is to implement.
> But here the question isn't even about printing, but only about accepting udata in getAsSignedConstant() and sdata in getAsUnsignedConstant(). Would you prefer the values to be accepted if they fit into the output range and return None otherwise?
Well, except that getAs(Un)signedConstant is used for dumping (it's also used for other stuff - like getting indexes into the file table... - anything else?).

I think it'd be perfectly reasonable if someone used DW_FORM_sdata for DW_AT_decl_line, to print the value appropriately signed, rather than printing hex bytes. Seems like a good/legible thing to do, even if the input is nonsense.

http://reviews.llvm.org/D5630






More information about the llvm-commits mailing list