<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 18 April 2014 14:24, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: dblaikie<br>
Date: Fri Apr 18 16:24:12 2014<br>
New Revision: 206653<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=206653&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=206653&view=rev</a><br>
Log:<br>
Update the fragments of symbols in compressed sections.<br>
<br>
While unnamed relocations are already cached in side tables in<br>
ELFObjectWriter::RecordRelocation, symbols still need their fragments<br>
updated to refer to the newly compressed fragment (even if that fragment<br>
isn't big enough to fit the offset). Even though we only create<br>
temporary symbols in debug info sections this comes up in 32 bit builds<br>
where even temporary symbols in mergeable sections (such as debug_str)<br>
have to be emitted as named symbols.<br>
<br>
I tried a few other ways to do this but they all didn't work for various<br>
reasons:<br>
<br>
1) Canonicalize the MCSymbolData in RecordRelocation, nulling out the<br>
Fragment (so it didn't have to be updated by CompressDebugSection). This<br>
doesn't work because some code relies on symbols having fragments to<br>
indicate that they're defined, I think.<br>
<br>
2) Canonicalize the MCSymbolData in RecordRelocation to be "first<br>
fragment + absolute offset" so it would be cheaper to just test and<br>
update the fragment in CompressDebugSections. This doesn't work because<br>
the offset computed in RecordRelocation isn't that of the symbol's<br>
fragment, it's the passed in fragment (I haven't figured out what that<br>
fragment is - perhaps it's the location where the relocation is to be<br>
written). And if the fragment offset has to be computed only for this<br>
use we might as well just do it when we need to, in<br>
CompressDebugSection.<br>
<br>
I also added an assert to help catch this a bit more clearly, even<br>
though it is UB. The test case improvements would either assert fail<br>
and/or valgrind vail without the fix, even if they wouldn't necessarily<br>
fail the FileCheck output.<br>
<br>
Modified:<br>
llvm/trunk/lib/MC/ELFObjectWriter.cpp<br>
llvm/trunk/test/MC/ELF/compression.s<br>
<br>
Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=206653&r1=206652&r2=206653&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=206653&r1=206652&r2=206653&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)<br>
+++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Fri Apr 18 16:24:12 2014<br>
@@ -616,6 +616,10 @@ static const MCSymbol *getBaseSymbol(con<br>
void ELFObjectWriter::WriteSymbol(SymbolTableWriter &Writer, ELFSymbolData &MSD,<br>
const MCAsmLayout &Layout) {<br>
MCSymbolData &OrigData = *MSD.SymbolData;<br>
+ assert(!OrigData.getFragment() ||<br>
+ (&OrigData.getFragment()->getParent()->getSection() ==<br>
+ &OrigData.getSymbol().getSection()) &&<br>
+ "The symbol's section doesn't match the fragment's symbol");<br></blockquote><div><br></div><div>GCC says:</div><div><br></div><div><div>lib/MC/ELFObjectWriter.cpp: In member function 'void {anonymous}::ELFObjectWriter::WriteSymbol({anonymous}::SymbolTableWriter&, {anonymous}::ELFObjectWriter::ELFSymbolData&, const llvm::MCAsmLayout&)':</div>
<div>lib/MC/ELFObjectWriter.cpp:621:47: error: suggest parentheses around '&&' within '||' [-Werror=parentheses]</div></div><div><br></div><div>That sounds reasonable to me. You've got assert(!x || (y && z) && "string!");</div>
<div><br></div><div>Nick</div><div><br></div></div></div></div>