<div dir="ltr">Agreed</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 26, 2017 at 2:42 PM Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">I don't think that any tool currently depends on parsing that syntax and I agree that using the mathematical [x,y) notation makes most sense here. It would be great to wrap this in a formatDWARFrange() function, so we can be consistent everywhere (and in case it really turns out that I need to produce darwin-dwarfdump compatible output at one point, we have one point where this can easily be parameterized).</div><div style="word-wrap:break-word"><div><br></div><div>-- adrian</div></div><div style="word-wrap:break-word"><div><br><div><blockquote type="cite"><div>On Apr 26, 2017, at 2:26 PM, Robinson, Paul <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>> wrote:</div><br class="m_-4645974411876234215Apple-interchange-newline"><div><div class="m_-4645974411876234215WordSection1" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I'd have a mild preference for [x, y) and also okay with x – y without the [) around the pair.  The [x – y) does just look funny. Although of course in context it's clear what's intended, so it's just a mild preference.<u></u><u></u></span></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">--paulr<u></u><u></u></span></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><a name="m_-4645974411876234215__MailEndCompose"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></a></div><div style="border-style:none none none solid;border-left-width:1.5pt;border-left-color:blue;padding:0in 0in 0in 4pt"><div><div style="border-style:solid none none;border-top-width:1pt;border-top-color:rgb(181,196,223);padding:3pt 0in 0in"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"><span class="m_-4645974411876234215Apple-converted-space"> </span>llvm-commits [<a href="mailto:llvm-commits-bounces@lists.llvm.org" target="_blank">mailto:llvm-commits-bounces@lists.llvm.org</a>]<span class="m_-4645974411876234215Apple-converted-space"> </span><b>On Behalf Of<span class="m_-4645974411876234215Apple-converted-space"> </span></b>David Blaikie via llvm-commits<br><b>Sent:</b><span class="m_-4645974411876234215Apple-converted-space"> </span>Wednesday, April 26, 2017 10:41 AM<br><b>To:</b><span class="m_-4645974411876234215Apple-converted-space"> </span><a href="mailto:reviews+D32492+public+d04ad76ab213883a@reviews.llvm.org" target="_blank">reviews+D32492+public+d04ad76ab213883a@reviews.llvm.org</a>; <a href="mailto:grimar@accesssoftek.com" target="_blank">grimar@accesssoftek.com</a>; <a href="mailto:dccitaliano@gmail.com" target="_blank">dccitaliano@gmail.com</a>; <a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>; Adrian Prantl<br><b>Cc:</b><span class="m_-4645974411876234215Apple-converted-space"> </span><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><b>Subject:</b><span class="m_-4645974411876234215Apple-converted-space"> </span>Re: [PATCH] D32492: [llvm-dwarfdump] - Change format for .gdb_index dump.<u></u><u></u></span></div></div></div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><u></u> <u></u></div><div><p class="MsoNormal" style="margin:0in 0in 12pt;font-size:12pt;font-family:'Times New Roman',serif"><u></u> <u></u></p><div><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif">On Wed, Apr 26, 2017 at 3:31 AM George Rimar via Phabricator <<a href="mailto:reviews@reviews.llvm.org" style="color:purple;text-decoration:underline" target="_blank">reviews@reviews.llvm.org</a>> wrote:<u></u><u></u></div></div><blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif">grimar added inline comments.<br><br><br>================<br>Comment at: lib/DebugInfo/DWARF/DWARFGdbIndex.cpp:42<br>     OS << format(<br>-        "    Low address = 0x%llx, High address = 0x%llx, CU index = %d\n",<br>-        Addr.LowAddress, Addr.HighAddress, Addr.CuIndex);<br>+        "    Low/High address = [0x%llx, 0x%llx] (Size: 0x%llx), CU id = %d\n",<br>+        Addr.LowAddress, Addr.HighAddress, Addr.HighAddress - Addr.LowAddress,<br>----------------<br>dblaikie wrote:<br>> While you're here, probably makes sense to fix the range to render as [x, y) rather than [x, y] ? (since it's a half open range, if I recall correctly)<br>I am not sure I understand why it is half open. If I got your idea correctly then probably output would be:<br>```<br>[a, b) (c, d) (e, f) (g, h]<u></u><u></u></div></blockquote><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><br>Nah, more like:<br><br>[a, b)<br>[c, d)<br>[e, f)<br>[g, h)<br> <u></u><u></u></div></div><blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif">```<br><br>But I do not think it would be correct.<br>.gdb_index format (<a href="https://sourceware.org/gdb/onlinedocs/gdb/Index-Section-Format.html" style="color:purple;text-decoration:underline" target="_blank">https://sourceware.org/gdb/onlinedocs/gdb/Index-Section-Format.html</a>) defines address area as:<br>> The address area. The address area consists of a sequence of address entries. Each address entry has three elements:<br>> The low address. This is a 64-bit little-endian value.<br>> The high address. This is a 64-bit little-endian value. Like DW_AT_high_pc, the value is one byte beyond the end.<u></u><u></u></div></blockquote><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><br>This bit here ^ "the value is one byte beyond the end" is what I mean by a half open range.<br><br>If a range in a range list includes bytes at offset/address 5, 6, 7, and 8. Then the range would be emitted as "5, 9" which is half open or written mathematically [5, 9)<br><br>( <a href="https://en.wikipedia.org/wiki/Interval_(mathematics)" style="color:purple;text-decoration:underline" target="_blank">https://en.wikipedia.org/wiki/Interval_(mathematics)</a> - "<span style="font-size:10.5pt;color:rgb(34,34,34)">A<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span><b>half-open interval</b><span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span>includes only one of its endpoints, and is denoted by mixing the notations for open and closed intervals.<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">(0,1]</span></span><span class="m_-4645974411876234215inbox-inbox-apple-converted-space"><span style="font-size:10.5pt;color:rgb(34,34,34)"> </span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">means greater than<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">0</span></span><span class="m_-4645974411876234215inbox-inbox-apple-converted-space"><span style="font-size:10.5pt;color:rgb(34,34,34)"> </span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">and less than or equal to<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">1</span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">, while<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">[0,1)</span></span><span class="m_-4645974411876234215inbox-inbox-apple-converted-space"><span style="font-size:10.5pt;color:rgb(34,34,34)"> </span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">means greater than or equal to<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">0</span></span><span class="m_-4645974411876234215inbox-inbox-apple-converted-space"><span style="font-size:10.5pt;color:rgb(34,34,34)"> </span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">and less than<span class="m_-4645974411876234215inbox-inbox-apple-converted-space"> </span></span><span class="m_-4645974411876234215inbox-inbox-texhtml"><span style="font-size:12.5pt;color:rgb(34,34,34)">1</span></span><span style="font-size:10.5pt;color:rgb(34,34,34)">.")</span><br> <u></u><u></u></div></div><blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif">> The CU index. This is an offset_type value.<br>><br><br>So we have set of elements that have begining and end marks. It looks to be set of closed segments/snippets, if I am not missing something, so I think [x, y] is more appropriate then.<br><br>I was interested how readelf dumps the .gdb_index for the same file used in testcase.<br>It was:<br>readelf --debug-dump=gdb_index dwarfdump-gdbindex-v7.elf-x86-64<br>```<br>Contents of the .gdb_index section:<br>Version 7<br><br>CU table:<br>[  0] 0x0 - 0x33<br>[  1] 0x34 - 0x67<br><br>TU table:<br><br>Address table:<br>00000000004000e8 00000000004000f3 0<br>00000000004000f3 00000000004000fe 1<br><br>Symbol table:<br>[489] main: 0 [global, function]<br>[754] int:<br>        0 [static, type]<br>        1 [static, type]<br>[956] main2: 1 [global, function]<br>```<br><br>Basing on above I can suggest 2 solutions:<br>1) Leave [x, y] as is.<br>2) Change dump output to something that avoids use of []() if that can be confusing, for example to:<br><br>```<br>Address area offset = 0x38, has 2 entries:<br>0x4000e8 - 0x4000f3, Size = 0xb, CU id = 0<br>0x4000f3 - 0x4000fe, Size = 0xb, CU id = 1<br>```<br><br>What do you think ?<u></u><u></u></div></blockquote><div><p class="MsoNormal" style="margin:0in 0in 12pt;font-size:12pt;font-family:'Times New Roman',serif"><br>'-' could be ambiguous with the minus sign here.<br><br>Indeed llvm-dwarfdump uses the half open range notation [x, y) in the DW_AT_ranges dumping such as here:<u></u><u></u></p><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><span style="font-family:'Courier New'">  DW_AT_ranges [DW_FORM_sec_offset] (0x00000000</span><u></u><u></u></div></div><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><span style="font-family:'Courier New'">     [0x00000000004004f0 - 0x00000000004004f6)</span><u></u><u></u></div></div><div><div style="margin:0in 0in 0.0001pt;font-size:12pt;font-family:'Times New Roman',serif"><span style="font-family:'Courier New'">     [0x0000000000400500 - 0x0000000000400508))</span><u></u><u></u></div></div><p class="MsoNormal" style="margin:0in 0in 12pt;font-size:12pt;font-family:'Times New Roman',serif"> <br>though it also uses - rather than ','... which is weird.<br><br>Adrian - what do you reckon we should standardize on here? I do sort of get the x - y syntax, but it seems a bit awkward when combined with the [x, y) syntax (which I also like, for clarity about the boundaries). I'd tend to lean towards [x, y)? But I could live with sticking with the [x - y)<u></u><u></u></p></div><blockquote style="border-style:none none none solid;border-left-width:1pt;border-left-color:rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><p class="MsoNormal" style="margin:0in 0in 12pt;font-size:12pt;font-family:'Times New Roman',serif"><br><br><a href="https://reviews.llvm.org/D32492" style="color:purple;text-decoration:underline" target="_blank">https://reviews.llvm.org/D32492</a></p></blockquote></div></div></div></div></div></blockquote></div><br></div></div></blockquote></div>