<html><head><base href="x-msg://15356/"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Danil,<div><br><div><div>On Nov 7, 2011, at 2:40 PM, Danil Malyshev wrote:<span class="Apple-style-span" style="color: rgb(0, 0, 128); font-family: Arial; font-size: 13px; "> </span></div><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">Please find attached the changed patch which addresses the following 2 things:<o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">1. Functions in ELFObjectFile, MachOObjectFile and COFFObjectFile should returns same result.</span></font><font color="black"></font></div></div></div></span></blockquote><div><br></div><div>This is fine in general, but we need to make sure we're standardizing on the <i>right</i> behaviors.</div><br><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font color="black"><span lang="EN-US" style="color: black; "><o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">2. We don't need have several functions with similar functionality which can be obtained by other means.</span></font></div></div></div></span></blockquote><div><br></div>I have many more issues with this one. Comments inline.</div><div><br><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">getSymbolAddress - removed.</span></font></div></div></div></span></blockquote><div><br></div><div>I don't like this. Getting symbol virtual addresses is one of the most common operations we do in, say, llvm-objdump.cpp or MachODump.cpp. I think this will make that code needlessly complex and non-obvious.</div><br><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font color="black"><span lang="EN-US" style="color: black; "><o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">getRelocationAddress - removed.</span></font></div></div></div></span></blockquote><div><br></div><div>This can't be removed. MachO scattered relocations relocate on a virtual address, rather than a symbol, so getSymbol().getAddress() doesn't work for them.</div><br><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">getSymbolOffset - returns the offset from the beginning of the section.</span></font><font color="black"></font></div></div></div></span></blockquote><div><br></div><div>I don't particularly <i>mind</i> standardizing on this as the definition for "getOffset" methods, but it doesn't seem particularly useful to me either. Why require a two-stage lookup (section offset + symbol offset from section) rather than just defining it to return a file offset?</div><br><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font color="black"><span lang="EN-US" style="color: black; "><o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">If we need the virtual address we can use something like symbol.getOffset() + symbol.getSection().getAddress()</span></font><font color="black"></font></div></div></div></span></blockquote><div><br></div><div>See my above comments about how common this lookup is and why I don't think we should make it <i>more</i> complex.</div><div><span class="Apple-style-span" style="font-family: 'Times New Roman'; font-size: 16px; "> </span></div><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; "><div lang="RU" link="blue" vlink="blue"><div class="Section1" style="page: Section1; "><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font color="black"><span lang="EN-US" style="color: black; "><o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">If we need the pointer to the first byte of symbol we can use something like getOffset() + getSection().getSectionContents().begin()</span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">getRelocationOffset - returns the offset from the beginning of the section.</span></font><font color="black"><span lang="EN-US" style="color: black; "><o:p></o:p></span></font></div><div style="margin-top: 0cm; margin-right: 0cm; margin-left: 0cm; margin-bottom: 0.0001pt; font-size: 12pt; font-family: 'Times New Roman'; "><font size="2" color="navy" face="Arial"><span lang="EN-US" style="font-size: 10pt; font-family: Arial; color: navy; ">If we need the virtual address we can use somethings like reloc.getOffset() + reloc.getSection().getAddress()</span></font><font color="black"></font></div></div></div></span></blockquote><div><br></div><div>This does not match my understanding of what getRelocationAddress() computes. My understanding is that it returns the virtual address of the value being relocated upon, in which case this is not the same as what you are proposing. See my comments above about why this value is necessary on MachO rather than <i>just</i> using getSymbol().getAddress().</div></div><br></div><div>--Owen</div></body></html>