[libunwind] r321440 - [libunwind] Avoid using C++ headers.

whitequark via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 25 12:43:55 PST 2017 

On 2017-12-25 20:32, Don Hinton wrote:
> 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.

That header is a header private to the libunwind implementation,
and the template function is confined to `namespace libunwind`.

> However, if you did want to do that, you should handle signed types as
> well, which pint_max_value doesn't do.

`pint_t` is an unsigned type in libunwind, the signed counterpart is 
`sint_t`.

> 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.

> 
> 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
>> [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 [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
>> [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 [4] [3]
>> [3]
>> 
>> Links:
>> ------
>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [5]
>> [4]
>> [2]
>> 
>> 
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [6]
>> [5]
>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [4]
>> [3]
>> 
>> --
>> whitequark
>> 
>> Links:
>> ------
>> [1] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev [5]
>> [2]
>> 
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
>> [6]
>> [3] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits [4]
>> [4] http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
>> [7]
>> [5]
>> 
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;r1=321439&amp;r2=321440&amp;view=diff
>> [8]
> 
> --
> whitequark
> 
> 
> 
> Links:
> ------
> [1]
> http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
> [2] http://llvm.org/viewvc/llvm-project?rev=321440&view=rev
> [3]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff
> [4] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> [5] http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev
> [6]
> http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;r1=321439&amp;r2=321440&amp;view=diff
> [7] http://llvm.org/viewvc/llvm-project?rev=321440&amp;amp;view=rev
> [8]
> 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