[libunwind] r321440 - [libunwind] Avoid using C++ headers.
Don Hinton via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 25 12:32:42 PST 2017
While beauty is in the eye of the beholder, I'm not sure introducing a new
template function in a header that's only used in that header is a good
idea. Every file that includes DwarfParser.hpp is going to get that
template function -- and someone may try to use it someday.
However, if you did want to do that, you should handle signed types as
well, which pint_max_value doesn't do. It should also assert for
non-integral types.
On Mon, Dec 25, 2017 at 12:06 PM, whitequark <whitequark at whitequark.org>
wrote:
> On 2017-12-25 19:43, Don Hinton wrote:
>
>> Here's the patch I applied locally.
>>
>> hth...
>> don
>> [snip]
>>
>
> I've committed a slightly beautified version of the patch (below)
> as r321446. Cheers!
>
> From 8a4760bafc1123f09438587ee5432eabdec3d33d Mon Sep 17 00:00:00 2001
> From: whitequark <whitequark at whitequark.org>
> Date: Mon, 25 Dec 2017 20:03:40 +0000
> Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.
>
> ---
> src/DwarfParser.hpp | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp
> index 518101e..645ac21 100644
> --- a/src/DwarfParser.hpp
> +++ b/src/DwarfParser.hpp
> @@ -17,7 +17,6 @@
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> -#include <limits.h>
>
> #include "libunwind.h"
> #include "dwarf2.h"
> @@ -26,6 +25,10 @@
>
> namespace libunwind {
>
> +// Avoid relying on C++ headers.
> +template <class T>
> +static constexpr T pint_max_value() { return ~0; }
> +
> /// CFI_Parser does basic parsing of a CFI (Call Frame Information)
> records.
> /// See DWARF Spec for details:
> /// http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
> -Core-generic/ehframechpt.html
> @@ -540,7 +543,7 @@ bool CFI_Parser<A>::parseInstructions(A
> &addressSpace, pint_t instructions,
> results->cfaRegister = 0;
> results->cfaExpression = (int64_t)p;
> length = addressSpace.getULEB128(p, instructionsEnd);
> - assert(length < std::numeric_limits<pint_t>::max() && "pointer
> overflow");
> + assert(length < pint_max_value<pint_t>() && "pointer overflow");
> p += static_cast<pint_t>(length);
> _LIBUNWIND_TRACE_DWARF("DW_CFA_def_cfa_expression(expression=0x%"
> PRIx64
> ", length=%" PRIu64 ")\n",
> @@ -556,7 +559,7 @@ bool CFI_Parser<A>::parseInstructions(A
> &addressSpace, pint_t instructions,
> results->savedRegisters[reg].location = kRegisterAtExpression;
> results->savedRegisters[reg].value = (int64_t)p;
> length = addressSpace.getULEB128(p, instructionsEnd);
> - assert(length < std::numeric_limits<pint_t>::max() && "pointer
> overflow");
> + assert(length < pint_max_value<pint_t>() && "pointer overflow");
> p += static_cast<pint_t>(length);
> _LIBUNWIND_TRACE_DWARF("DW_CFA_expression(reg=%" PRIu64 ", "
> "expression=0x%" PRIx64 ", "
> @@ -642,7 +645,7 @@ bool CFI_Parser<A>::parseInstructions(A
> &addressSpace, pint_t instructions,
> results->savedRegisters[reg].location = kRegisterIsExpression;
> results->savedRegisters[reg].value = (int64_t)p;
> length = addressSpace.getULEB128(p, instructionsEnd);
> - assert(length < std::numeric_limits<pint_t>::max() && "pointer
> overflow");
> + assert(length < pint_max_value<pint_t>() && "pointer overflow");
> p += static_cast<pint_t>(length);
> _LIBUNWIND_TRACE_DWARF("DW_CFA_val_expression(reg=%" PRIu64 ", "
> "expression=0x%" PRIx64 ", length=%" PRIu64
> ")\n",
> --
> 2.11.0
>
>
>> On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton <hintonda at gmail.com>
>> wrote:
>>
>> On Mon, Dec 25, 2017 at 11:09 AM, whitequark
>>> <whitequark at whitequark.org> wrote:
>>> On 2017-12-25 19:04, Don Hinton wrote:
>>> Hi:
>>>
>>> This change breaks in a local debug build, e.g.,:
>>>
>>>
>>> /Users/dhinton/projects/llvm_project/libunwind/src/DwarfPars
>> er.hpp:559:28:
>>
>>> error: no member named 'numeric_limits' in namespace 'std'
>>> assert(length < std::numeric_limits<pint_t>::max() && "pointer
>>> overflow");
>>> ~~~~~^
>>>
>>> Sorry, I missed this. Any idea on reformulating the assert in a way
>>> that does not require libcxx headers? Not having them significantly
>>> simplifies bare-metal builds...
>>>
>>
>> Well, assuming pint_t is some unsigned integer type, the max can be
>> found like this:
>>
>> pint_t max_pint_t = ~0;
>>
>> So, that could be used in a pinch.
>>
>> thanks...
>>> don
>>>
>>> On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits
>>> <cfe-commits at lists.llvm.org> wrote:
>>>
>>> Author: whitequark
>>> Date: Mon Dec 25 05:06:09 2017
>>> New Revision: 321440
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [1]
>>> [1]
>>> Log:
>>> [libunwind] Avoid using C++ headers.
>>>
>>> This is useful for building libunwind on libcxx-free systems.
>>>
>>> Modified:
>>> libunwind/trunk/src/DwarfParser.hpp
>>>
>>> Modified: libunwind/trunk/src/DwarfParser.hpp
>>> URL:
>>>
>>>
>>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>>
>>> [2]
>>> [2]
>>>
>>>
>>> ============================================================
>> ==================
>>
>>> --- libunwind/trunk/src/DwarfParser.hpp (original)
>>> +++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 25 05:06:09 2017
>>> @@ -17,7 +17,7 @@
>>> #include <stdint.h>
>>> #include <stdio.h>
>>> #include <stdlib.h>
>>> -#include <limits>
>>> +#include <limits.h>
>>>
>>> #include "libunwind.h"
>>> #include "dwarf2.h"
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3] [3]
>>>
>>> Links:
>>> ------
>>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [4]
>>> [2]
>>>
>>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>>
>>> [5]
>>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [3]
>>>
>>
>> --
>> whitequark
>>
>>
>>
>> Links:
>> ------
>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev
>> [2]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> [4] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev
>> [5]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=32144
>> 0&view=diff
>>
>
> --
> whitequark
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171225/35e1e472/attachment-0001.html>
More information about the cfe-commits
mailing list