[libunwind] r321440 - [libunwind] Avoid using C++ headers.
whitequark via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 25 13:10:13 PST 2017
On 2017-12-25 20:54, Don Hinton wrote:
> On Mon, Dec 25, 2017 at 12:43 PM, whitequark
> <whitequark at 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!
>
> 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
>> <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
> [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 <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/DwarfParser.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]
>> [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/DwarfParser.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 at 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/DwarfParser.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/DwarfParser.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&view=rev
>> [4]
>> [7]
>> [5]
>
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&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/DwarfParser.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&view=rev
> [4]
> [6]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
> [8]
> [7]
> http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
> [9]
> [8]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;r1=321439&amp;r2=321440&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&view=rev
> [4] http://llvm.org/viewvc/llvm-project?rev=321440&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/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
> [7]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
> [8]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;r1=321439&amp;r2=321440&amp;view=diff
> [9]
> http://llvm.org/viewvc/llvm-project?rev=321440&amp;amp;view=rev
> [10]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;amp;r1=321439&amp;amp;r2=321440&amp;amp;view=diff
--
whitequark
More information about the cfe-commits
mailing list