<div dir="ltr">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 <span style="font-size:12.8px">DwarfParser.hpp is going to get that template function -- and someone may try to use it someday.</span><div><br></div><div>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.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 25, 2017 at 12:06 PM, whitequark <span dir="ltr"><<a href="mailto:whitequark@whitequark.org" target="_blank">whitequark@whitequark.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 2017-12-25 19:43, Don Hinton wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
Here's the patch I applied locally.<br>
<br>
hth...<br>
don<br></span>
[snip]<br>
</blockquote>
<br>
I've committed a slightly beautified version of the patch (below)<br>
as r321446. Cheers!<br>
<br>
>From 8a4760bafc1123f09438587ee5432e<wbr>abdec3d33d Mon Sep 17 00:00:00 2001<br>
From: whitequark <<a href="mailto:whitequark@whitequark.org" target="_blank">whitequark@whitequark.org</a>><br>
Date: Mon, 25 Dec 2017 20:03:40 +0000<br>
Subject: [PATCH] [libunwind] Unbreak debug builds after r321440.<br>
<br>
---<br>
src/DwarfParser.hpp | 11 +++++++----<br>
1 file changed, 7 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/DwarfParser.hpp b/src/DwarfParser.hpp<br>
index 518101e..645ac21 100644<br>
--- a/src/DwarfParser.hpp<br>
+++ b/src/DwarfParser.hpp<br>
@@ -17,7 +17,6 @@<span class=""><br>
#include <stdint.h><br>
#include <stdio.h><br>
#include <stdlib.h><br></span>
-#include <limits.h><br>
<br>
#include "libunwind.h"<br>
#include "dwarf2.h"<br>
@@ -26,6 +25,10 @@<br>
<br>
namespace libunwind {<br>
<br>
+// Avoid relying on C++ headers.<br>
+template <class T><br>
+static constexpr T pint_max_value() { return ~0; }<br>
+<br>
/// CFI_Parser does basic parsing of a CFI (Call Frame Information) records.<br>
/// See DWARF Spec for details:<br>
/// <a href="http://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html" rel="noreferrer" target="_blank">http://refspecs.linuxbase.org/<wbr>LSB_3.1.0/LSB-Core-generic/LSB<wbr>-Core-generic/ehframechpt.html</a><br>
@@ -540,7 +543,7 @@ bool CFI_Parser<A>::parseInstructio<wbr>ns(A &addressSpace, pint_t instructions,<span class=""><br>
results->cfaRegister = 0;<br>
results->cfaExpression = (int64_t)p;<br>
length = addressSpace.getULEB128(p, instructionsEnd);<br>
- assert(length < std::numeric_limits<pint_t>::m<wbr>ax() && "pointer overflow");<br></span>
+ assert(length < pint_max_value<pint_t>() && "pointer overflow");<span class=""><br>
p += static_cast<pint_t>(length);<br>
_LIBUNWIND_TRACE_DWARF("DW_CF<wbr>A_def_cfa_expression(expressio<wbr>n=0x%" PRIx64<br>
", length=%" PRIu64 ")\n",<br></span>
@@ -556,7 +559,7 @@ bool CFI_Parser<A>::parseInstructio<wbr>ns(A &addressSpace, pint_t instructions,<span class=""><br>
results->savedRegisters[reg].<wbr>location = kRegisterAtExpression;<br>
results->savedRegisters[reg].<wbr>value = (int64_t)p;<br>
length = addressSpace.getULEB128(p, instructionsEnd);<br>
- assert(length < std::numeric_limits<pint_t>::m<wbr>ax() && "pointer overflow");<br></span>
+ assert(length < pint_max_value<pint_t>() && "pointer overflow");<span class=""><br>
p += static_cast<pint_t>(length);<br>
_LIBUNWIND_TRACE_DWARF("DW_CF<wbr>A_expression(reg=%" PRIu64 ", "<br>
"expression=0x%" PRIx64 ", "<br></span>
@@ -642,7 +645,7 @@ bool CFI_Parser<A>::parseInstructio<wbr>ns(A &addressSpace, pint_t instructions,<span class=""><br>
results->savedRegisters[reg].<wbr>location = kRegisterIsExpression;<br>
results->savedRegisters[reg].<wbr>value = (int64_t)p;<br>
length = addressSpace.getULEB128(p, instructionsEnd);<br>
- assert(length < std::numeric_limits<pint_t>::m<wbr>ax() && "pointer overflow");<br></span>
+ assert(length < pint_max_value<pint_t>() && "pointer overflow");<span class=""><br>
p += static_cast<pint_t>(length);<br>
_LIBUNWIND_TRACE_DWARF("DW_CF<wbr>A_val_expression(reg=%" PRIu64 ", "<br>
"expression=0x%" PRIx64 ", length=%" PRIu64 ")\n",<br></span>
-- <br>
2.11.0<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
On Mon, Dec 25, 2017 at 11:26 AM, Don Hinton <<a href="mailto:hintonda@gmail.com" target="_blank">hintonda@gmail.com</a>><br>
wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Dec 25, 2017 at 11:09 AM, whitequark<br>
<<a href="mailto:whitequark@whitequark.org" target="_blank">whitequark@whitequark.org</a>> wrote:<br>
On 2017-12-25 19:04, Don Hinton wrote:<br>
Hi:<br>
<br>
This change breaks in a local debug build, e.g.,:<br>
<br>
<br>
</blockquote>
/Users/dhinton/projects/llvm_p<wbr>roject/libunwind/src/DwarfPars<wbr>er.hpp:559:28:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
error: no member named 'numeric_limits' in namespace 'std'<br>
assert(length < std::numeric_limits<pint_t>::m<wbr>ax() && "pointer<br>
overflow");<br>
~~~~~^<br>
<br>
Sorry, I missed this. Any idea on reformulating the assert in a way<br>
that does not require libcxx headers? Not having them significantly<br>
simplifies bare-metal builds...<br>
</blockquote>
<br>
Well, assuming pint_t is some unsigned integer type, the max can be<br>
found like this:<br>
<br>
pint_t max_pint_t = ~0;<br>
<br>
So, that could be used in a pinch.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
thanks...<br>
don<br>
<br>
On Mon, Dec 25, 2017 at 5:06 AM, whitequark via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
<br>
Author: whitequark<br>
Date: Mon Dec 25 05:06:09 2017<br>
New Revision: 321440<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321440&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321440&view=rev</a> [1]<br>
[1]<br>
Log:<br>
[libunwind] Avoid using C++ headers.<br>
<br>
This is useful for building libunwind on libcxx-free systems.<br>
<br>
Modified:<br>
libunwind/trunk/src/DwarfParse<wbr>r.hpp<br>
<br>
Modified: libunwind/trunk/src/DwarfParse<wbr>r.hpp<br>
URL:<br>
<br>
<br>
</blockquote>
<a href="http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/libunwind/trunk/src/Dwar<wbr>fParser.hpp?rev=321440&r1=3214<wbr>39&r2=321440&view=diff</a><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[2]<br>
[2]<br>
<br>
<br>
</blockquote>
==============================<wbr>==============================<wbr>==================<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
--- libunwind/trunk/src/DwarfParse<wbr>r.hpp (original)<br>
+++ libunwind/trunk/src/DwarfParse<wbr>r.hpp Mon Dec 25 05:06:09 2017<br>
@@ -17,7 +17,7 @@<br>
#include <stdint.h><br>
#include <stdio.h><br>
#include <stdlib.h><br>
-#include <limits><br>
+#include <limits.h><br>
<br>
#include "libunwind.h"<br>
#include "dwarf2.h"<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
</div></div><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a> [3] [3]<br>
<br>
Links:<br>
------<br>
[1] <a href="http://llvm.org/viewvc/llvm-project?rev=321440&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321440&view=rev</a> [4]<br>
[2]<br>
<br>
</blockquote><span class="">
<a href="http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/libunwind/trunk/src/Dwar<wbr>fParser.hpp?rev=321440&r1=<wbr>321439&r2=321440&view=<wbr>diff</a><br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
[5]<br>
[3] <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a> [3]<br>
</blockquote>
<br>
--<br>
whitequark<span class=""><br>
<br>
<br>
<br>
Links:<br>
------<br>
[1] <a href="http://llvm.org/viewvc/llvm-project?rev=321440&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321440&view=rev</a><br>
[2]<br>
<a href="http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&r1=321439&r2=321440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/libunwind/trunk/src/Dwar<wbr>fParser.hpp?rev=321440&r1=<wbr>321439&r2=321440&view=<wbr>diff</a><br>
[3] <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br></span>
[4] <a href="http://llvm.org/viewvc/llvm-project?rev=321440&amp;view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=321440&amp;view=<wbr>rev</a><br>
[5]<br>
<a href="http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=321440&amp;r1=321439&amp;r2=321440&amp;view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/libunwind/trunk/src/Dwar<wbr>fParser.hpp?rev=321440&<wbr>amp;r1=321439&amp;r2=32144<wbr>0&amp;view=diff</a><span class="HOEnZb"><font color="#888888"><br>
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
-- <br>
whitequark<br>
</font></span></blockquote></div><br></div>