On Mon, Dec 25, 2017 at 1:10 PM, whitequark <whitequ...@whitequark.org>
wrote:
> On 2017-12-25 20:54, Don Hinton wrote:
>
>> On Mon, Dec 25, 2017 at 12:43 PM, whitequark
>> <whitequ...@whitequark.org> wrote:
>>
>>> On 2017-12-25 20:32, Don Hinton wrote:
>>>
>>>> It should also assert for non-integral types.
>>>>
>>>
>>> True, but I do not know enough C++ magic to implement that. AIUI
>>> std::numeric_limits implements this by specializing a template for
>>> every
>>> integral type, which is not something I will do here. Of course
>>> there's a standard template for checking whether a type is integral,
>>> but that also lives in libcxx.
>>>
>>
>> Why not just use static_cast<pint_t>(~0) and avoid all these issues?
>>
>
> r321448. Thanks for the review!
>
LGTM, thanks...
>
>
>> You might have a point that this doesn't apply in your case, but it's
>> a good habit to get into.
>>
>> On Mon, Dec 25, 2017 at 12:06 PM, whitequark
>>> <whitequ...@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 <whitequ...@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
>> [5]
>>
>>
>> [1]
>>>
>>> @@ -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 <hinto...@gmail.com>
>>> wrote:
>>>
>>> On Mon, Dec 25, 2017 at 11:09 AM, whitequark
>>> <whitequ...@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/DwarfPar
>> ser.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@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]
>>> [2] [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
>> [6]
>>
>> [3]
>>> [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@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [2] [4]
>>> [3]
>>> [3]
>>>
>>> Links:
>>> ------
>>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [3]
>>> [5]
>>> [4]
>>> [2]
>>>
>>
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [7]
>>
>> [6]
>>> [5]
>>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [2]
>>> [4]
>>> [3]
>>>
>>> --
>>> whitequark
>>>
>>> Links:
>>> ------
>>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [3]
>>> [5]
>>> [2]
>>>
>>
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [7]
>>
>> [6]
>>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [2]
>>> [4]
>>> [4] http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
>>> [4]
>>> [7]
>>> [5]
>>>
>>
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;r1=321439&amp;r2=32144
>> 0&amp;view=diff
>> [8]
>>
>> [8]
>>>
>>
>> --
>> whitequark
>>
>> Links:
>> ------
>> [1]
>> http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
>> -Core-generic/ehframechpt.html
>> [5]
>> [2] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [3]
>> [3]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [7]
>> [4] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [2]
>> [5] http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
>> [4]
>> [6]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;r1=321439&amp;r2=32144
>> 0&amp;view=diff
>> [8]
>> [7]
>> http://llvm.org/viewvc/llvm-project?rev=321440&amp;amp;view=rev
>> [9]
>> [8]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;amp;r1=321439&amp;amp;
>> r2=321440&amp;amp;view=diff
>> [10]
>>
>> --
>> whitequark
>>
>>
>>
>> Links:
>> ------
>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev
>> [2] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> [3] http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
>> [4] http://llvm.org/viewvc/llvm-project?rev=321440&amp;amp;view=rev
>> [5]
>> http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB
>> -Core-generic/ehframechpt.html
>> [6]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [7]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;r1=321439&amp;r2=32144
>> 0&amp;view=diff
>> [8]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;amp;r1=321439&amp;amp;
>> r2=321440&amp;amp;view=diff
>> [9] http://llvm.org/viewvc/llvm-project?rev=321440&amp;amp;
>> amp;view=rev
>> [10]
>> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Dwar
>> fParser.hpp?rev=321440&amp;amp;amp;r1=321439&amp;
>> amp;amp;r2=321440&amp;amp;amp;view=diff
>>
>
> --
> whitequark
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
