[llvm] r206653 - Update the fragments of symbols in compressed sections.

[Nick Lewycky]

On 18 April 2014 14:24, David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Fri Apr 18 16:24:12 2014
> New Revision: 206653
>
> URL: http://llvm.org/viewvc/llvm-project?rev=206653&view=rev
> Log:
> Update the fragments of symbols in compressed sections.
>
> While unnamed relocations are already cached in side tables in
> ELFObjectWriter::RecordRelocation, symbols still need their fragments
> updated to refer to the newly compressed fragment (even if that fragment
> isn't big enough to fit the offset). Even though we only create
> temporary symbols in debug info sections this comes up in 32 bit builds
> where even temporary symbols in mergeable sections (such as debug_str)
> have to be emitted as named symbols.
>
> I tried a few other ways to do this but they all didn't work for various
> reasons:
>
> 1) Canonicalize the MCSymbolData in RecordRelocation, nulling out the
> Fragment (so it didn't have to be updated by CompressDebugSection). This
> doesn't work because some code relies on symbols having fragments to
> indicate that they're defined, I think.
>
> 2) Canonicalize the MCSymbolData in RecordRelocation to be "first
> fragment + absolute offset" so it would be cheaper to just test and
> update the fragment in CompressDebugSections. This doesn't work because
> the offset computed in RecordRelocation isn't that of the symbol's
> fragment, it's the passed in fragment (I haven't figured out what that
> fragment is - perhaps it's the location where the relocation is to be
> written). And if the fragment offset has to be computed only for this
> use we might as well just do it when we need to, in
> CompressDebugSection.
>
> I also added an assert to help catch this a bit more clearly, even
> though it is UB. The test case improvements would either assert fail
> and/or valgrind vail without the fix, even if they wouldn't necessarily
> fail the FileCheck output.
>
> Modified:
>     llvm/trunk/lib/MC/ELFObjectWriter.cpp
>     llvm/trunk/test/MC/ELF/compression.s
>
> Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=206653&r1=206652&r2=206653&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
> +++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Fri Apr 18 16:24:12 2014
> @@ -616,6 +616,10 @@ static const MCSymbol *getBaseSymbol(con
>  void ELFObjectWriter::WriteSymbol(SymbolTableWriter &Writer,
> ELFSymbolData &MSD,
>                                    const MCAsmLayout &Layout) {
>    MCSymbolData &OrigData = *MSD.SymbolData;
> +  assert(!OrigData.getFragment() ||
> +         (&OrigData.getFragment()->getParent()->getSection() ==
> +          &OrigData.getSymbol().getSection()) &&
> +             "The symbol's section doesn't match the fragment's symbol");
>

GCC says:

lib/MC/ELFObjectWriter.cpp: In member function 'void
{anonymous}::ELFObjectWriter::WriteSymbol({anonymous}::SymbolTableWriter&,
{anonymous}::ELFObjectWriter::ELFSymbolData&, const llvm::MCAsmLayout&)':
lib/MC/ELFObjectWriter.cpp:621:47: error: suggest parentheses around '&&'
within '||' [-Werror=parentheses]

That sounds reasonable to me. You've got assert(!x || (y && z) &&
"string!");

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140418/787da48b/attachment.html>