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
<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
[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/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@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@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
