[llvm] r221835 - Allow DWARFFormValue::extractValue to be called with a null CU.
Frédéric Riss
friss at apple.com
Wed Nov 12 16:50:37 PST 2014
> On Nov 12, 2014, at 4:42 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Nov 12, 2014 at 4:06 PM, Frédéric Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>
>> On Nov 12, 2014, at 3:59 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>>
>>
>>
>> On Wed, Nov 12, 2014 at 3:48 PM, Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
>> Author: friss
>> Date: Wed Nov 12 17:48:04 2014
>> New Revision: 221835
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=221835&view=rev <http://llvm.org/viewvc/llvm-project?rev=221835&view=rev>
>> Log:
>> Allow DWARFFormValue::extractValue to be called with a null CU.
>>
>> Currently FormValues are only used for attributes of DIEs and thus
>> uers always have a CU lying around when calling into the FormValue
>> API.
>> Accelerator tables encode their information using the same Forms
>> as the attributes, thus it is natural to use DWARFFormValue to
>> extract/dump them. There is no CU in that case though. Allow the
>> API to be called with a null CU arguemnt by making the RelocMap
>> lookup conditional on the CU pointer validity. And document this
>> new behvior in the header. (Test coverage for this use of the API
>> comes in the DwarfAccelTable support patch)
>>
>> Modified:
>> llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h
>> llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp
>>
>> Modified: llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h?rev=221835&r1=221834&r2=221835&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h?rev=221835&r1=221834&r2=221835&view=diff>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h (original)
>> +++ llvm/trunk/include/llvm/DebugInfo/DWARFFormValue.h Wed Nov 12 17:48:04 2014
>> @@ -57,6 +57,13 @@ public:
>> bool isFormClass(FormClass FC) const;
>>
>> void dump(raw_ostream &OS, const DWARFUnit *U) const;
>> +
>> + /// \brief extracts a value in data at offset *offset_ptr.
>> + ///
>> + /// The passed DWARFUnit is allowed to be nullptr,
>>
>> "to be null" (nullptr is just one null pointer literal - "null" is the general concept)
>
> OK, I’ll fix that up.
>
>> in which
>> + /// case no relocation processing will be performed and some
>> + /// kind of forms that depend on Unit information are disallowed.
>>
>> Disallowed is a bit vague - elegant failure? Assertion? I would probably prefer assertion failure, if that's reasonable/possible.
>
> With this patch, the extractValue fails, ie. it returns false.
> I prefer that over the assertion,
>
> Why do you prefer that over an assertion? I assume that encoding isn't valid in the accelerator table - is the table self-describing (does it include the form code in the data read from the file? So it could, in theory (if mangled/fuzzed/etc) include a relocatable form?)? In that case I get where you're coming from, we shouldn't crash/assert on the bytes read from the file no matter how absurd they are.
Yes, exactly. Moreover, the function prototype was already tailored to return success/failure, thus adding that as another failure more seemed natural.
> but then disallowed is misleading. I should maybe reword that as "will result in an extraction failure”.
>
> Fred
>
>> + /// \returns wether the extraction succeeded.
>> bool extractValue(DataExtractor data, uint32_t *offset_ptr,
>> const DWARFUnit *u);
>> bool isInlinedCStr() const {
>>
>> Modified: llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp?rev=221835&r1=221834&r2=221835&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp?rev=221835&r1=221834&r2=221835&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp (original)
>> +++ llvm/trunk/lib/DebugInfo/DWARFFormValue.cpp Wed Nov 12 17:48:04 2014
>> @@ -139,6 +139,8 @@ bool DWARFFormValue::extractValue(DataEx
>> switch (Form) {
>> case DW_FORM_addr:
>> case DW_FORM_ref_addr: {
>> + if (!cu)
>> + return false;
>> uint16_t AddrSize =
>> (Form == DW_FORM_addr)
>> ? cu->getAddressByteSize()
>> @@ -179,8 +181,10 @@ bool DWARFFormValue::extractValue(DataEx
>> break;
>> case DW_FORM_data4:
>> case DW_FORM_ref4: {
>> - RelocAddrMap::const_iterator AI = cu->getRelocMap()->find(*offset_ptr);
>> Value.uval = data.getU32(offset_ptr);
>> + if (!cu)
>> + break;
>> + RelocAddrMap::const_iterator AI = cu->getRelocMap()->find(*offset_ptr-4);
>> if (AI != cu->getRelocMap()->end())
>> Value.uval += AI->second.second;
>> break;
>> @@ -193,13 +197,12 @@ bool DWARFFormValue::extractValue(DataEx
>> Value.sval = data.getSLEB128(offset_ptr);
>> break;
>> case DW_FORM_strp: {
>> - RelocAddrMap::const_iterator AI
>> - = cu->getRelocMap()->find(*offset_ptr);
>> - if (AI != cu->getRelocMap()->end()) {
>> - const std::pair<uint8_t, int64_t> &R = AI->second;
>> - Value.uval = data.getU32(offset_ptr) + R.second;
>> - } else
>> - Value.uval = data.getU32(offset_ptr);
>> + Value.uval = data.getU32(offset_ptr);
>> + if (!cu)
>> + break;
>> + RelocAddrMap::const_iterator AI = cu->getRelocMap()->find(*offset_ptr-4);
>> + if (AI != cu->getRelocMap()->end())
>> + Value.uval += AI->second.second;
>> break;
>> }
>> case DW_FORM_udata:
>> @@ -215,13 +218,12 @@ bool DWARFFormValue::extractValue(DataEx
>> break;
>> case DW_FORM_sec_offset: {
>> // FIXME: This is 64-bit for DWARF64.
>> - RelocAddrMap::const_iterator AI
>> - = cu->getRelocMap()->find(*offset_ptr);
>> - if (AI != cu->getRelocMap()->end()) {
>> - const std::pair<uint8_t, int64_t> &R = AI->second;
>> - Value.uval = data.getU32(offset_ptr) + R.second;
>> - } else
>> - Value.uval = data.getU32(offset_ptr);
>> + Value.uval = data.getU32(offset_ptr);
>> + if (!cu)
>> + break;
>> + RelocAddrMap::const_iterator AI = cu->getRelocMap()->find(*offset_ptr-4);
>> + if (AI != cu->getRelocMap()->end())
>> + Value.uval += AI->second.second;
>> break;
>> }
>> case DW_FORM_flag_present:
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141112/2a74fd0f/attachment.html>
More information about the llvm-commits
mailing list