[llvm] r173885 - This patch implements runtime ARM specific

[Carter, Jack]

Chandler,

Could you elaborate on "there is usually a better pattern to follow"?

I don't like the casts either and would prefer another way.

Jack

________________________________
From: Chandler Carruth [chandlerc at google.com]
Sent: Thursday, January 31, 2013 3:46 PM
To: Carter, Jack
Cc: Commit Messages and Patches for LLVM
Subject: Re: [llvm] r173885 - This patch implements runtime ARM specific
     // 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/20130201/5cf63669/attachment.html>