[llvm] r173885 - This patch implements runtime ARM specific

[Chandler Carruth]

On Tue, Jan 29, 2013 at 6:24 PM, Jack Carter <jcarter at mips.com> wrote:

> Author: jacksprat
> Date: Tue Jan 29 20:24:33 2013
> New Revision: 173885
>
> URL: http://llvm.org/viewvc/llvm-project?rev=173885&view=rev
> Log:
> This patch implements runtime ARM specific
> setting of ELF header e_flags.
>
> Contributer: Jack Carter
>
>
> Added:
>     llvm/trunk/test/MC/ARM/elf-eflags-eabi-cg.ll
> Modified:
>     llvm/trunk/include/llvm/Support/ELF.h
>     llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
>     llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
>     llvm/trunk/test/MC/ARM/elf-eflags-eabi.s
>
> Modified: llvm/trunk/include/llvm/Support/ELF.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ELF.h?rev=173885&r1=173884&r2=173885&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/ELF.h (original)
> +++ llvm/trunk/include/llvm/Support/ELF.h Tue Jan 29 20:24:33 2013
> @@ -495,7 +495,15 @@ enum {
>  };
>
>  // ARM Specific e_flags
> -enum { EF_ARM_EABIMASK = 0xFF000000U };
> +enum {
> +  EF_ARM_EABI_UNKNOWN =   0x00000000U,
> +  EF_ARM_EABI_VER1 =      0x01000000U,
> +  EF_ARM_EABI_VER2 =      0x02000000U,
> +  EF_ARM_EABI_VER3 =      0x03000000U,
> +  EF_ARM_EABI_VER4 =      0x04000000U,
> +  EF_ARM_EABI_VER5 =      0x05000000U,
> +  EF_ARM_EABIMASK =       0xFF000000U
> +};
>
>  // ELF Relocation types for ARM
>  // Meets 2.08 ABI Specs.
>
> Modified: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp?rev=173885&r1=173884&r2=173885&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Tue Jan 29 20:24:33 2013
> @@ -37,6 +37,7 @@
>  #include "llvm/MC/MCAsmInfo.h"
>  #include "llvm/MC/MCAssembler.h"
>  #include "llvm/MC/MCContext.h"
> +#include "llvm/MC/MCELFStreamer.h"
>  #include "llvm/MC/MCInst.h"
>  #include "llvm/MC/MCInstBuilder.h"
>  #include "llvm/MC/MCObjectStreamer.h"
> @@ -45,6 +46,7 @@
>  #include "llvm/MC/MCSymbol.h"
>  #include "llvm/Support/CommandLine.h"
>  #include "llvm/Support/Debug.h"
> +#include "llvm/Support/ELF.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/TargetRegistry.h"
>  #include "llvm/Support/raw_ostream.h"
> @@ -699,6 +701,15 @@ void ARMAsmPrinter::EmitEndOfAsmFile(Mod
>      // generates code that does this, it is always safe to set.
>      OutStreamer.EmitAssemblerFlag(MCAF_SubsectionsViaSymbols);
>    }
> +  // FIXME: This should eventually end up somewhere else where more
> +  // intelligent flag decisions can be made. For now we are just
> maintaining
> +  // the status quo for ARM and setting EF_ARM_EABI_VER5 as the default.
> +  if (Subtarget->isTargetELF()) {
> +    if (OutStreamer.hasRawTextSupport()) return;
> +
> +    MCELFStreamer &MES = static_cast<MCELFStreamer &>(OutStreamer);
> +    MES.getAssembler().setELFHeaderEFlags(ELF::EF_ARM_EABI_VER5);
> +  }
>  }
>
>
>  //===----------------------------------------------------------------------===//
>
> Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=173885&r1=173884&r2=173885&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Tue Jan 29
> 20:24:33 2013
> @@ -18,7 +18,9 @@
>  #include "llvm/ADT/StringSwitch.h"
>  #include "llvm/ADT/Twine.h"
>  #include "llvm/MC/MCAsmInfo.h"
> +#include "llvm/MC/MCAssembler.h"
>  #include "llvm/MC/MCContext.h"
> +#include "llvm/MC/MCELFStreamer.h"
>  #include "llvm/MC/MCExpr.h"
>  #include "llvm/MC/MCInst.h"
>  #include "llvm/MC/MCInstrDesc.h"
> @@ -28,6 +30,7 @@
>  #include "llvm/MC/MCRegisterInfo.h"
>  #include "llvm/MC/MCStreamer.h"
>  #include "llvm/MC/MCSubtargetInfo.h"
> +#include "llvm/Support/ELF.h"
>  #include "llvm/Support/MathExtras.h"
>  #include "llvm/Support/SourceMgr.h"
>  #include "llvm/Support/TargetRegistry.h"
> @@ -250,6 +253,13 @@ public:
>
>      // Not in an ITBlock to start with.
>      ITState.CurPosition = ~0U;
> +
> +    // Set ELF header flags.
> +    // FIXME: This should eventually end up somewhere else where more
> +    // intelligent flag decisions can be made. For now we are just
> maintaining
> +    // the status quo for ARM and setting EF_ARM_EABI_VER5 as the default.
> +    MCELFStreamer &MES = static_cast<MCELFStreamer
> &>(Parser.getStreamer());
> +    MES.getAssembler().setELFHeaderEFlags(ELF::EF_ARM_EABI_VER5);
>

Here, you don't test or check anything, you simply cast to MCELFStreamer,
and run. This caused *really* strange bugs. I've fixed it in r174118 to use
a proper check on the type of the MCStreamer here, but I would encourage
you to audit some of the other changes in this area to make sure there
aren't other instances of this bug.

In general, any use of static_cast<> to move through a dynamic class
hierarchy in LLVM is pretty suspect, and something to investigate as there
is usually a better pattern to follow.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130131/ac5e213a/attachment.html>