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 <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 > @@ -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/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@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@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&amp;view=rev >> [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 >> > > -- > whitequark >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits