[mips] Save a copy of MipsABIInfo in the MipsTargetStreamer to escape a dangling pointer

[atanasyan]

Summary

The MipsTargetELFStreamer can receive ABI info from many sources. For example, from the MipsAsmParser instance. Lifetime of the MipsAsmParser can be shorter than MipsTargetELFStreamer's lifetime. In that case we get a dangling pointer to MipsABIInfo.

[dsanders]

I think I can probably guess (direct object emission?) but can you tell me what situation causes MipsELFTargetStreamer to out live MipsAsmParser?

[atanasyan]

In D12805#244314, @dsanders wrote:

I think I can probably guess (direct object emission?) but can you tell me what situation causes MipsELFTargetStreamer to out live MipsAsmParser?

The following test case reproduces the problem. I am going to commit it to the clang repository after this fix.

// REQUIRES: mips-registered-target                                                                                                                                                                                
// RUN: %clang_cc1 -triple mips-linux-gnu -emit-obj -o - %s | \                                                                                                                                                    
// RUN:   llvm-readobj -h - | FileCheck %s                                                                                                                                                                           
                                                                                                                                                                                                                   
// CHECK: EF_MIPS_ABI_O32                                                                                                                                                                                          
                                                                                                                                                                                                                   
__asm__(                                                                                                                                                                                                           
"bar:\n"                                                                                                                                                                                                           
"  nop\n"                                                                                                                                                                                                          
);                                                                                                                                                                                                                 
                                                                                                                                                                                                                   
void foo() {}

[dsanders]

Hmm. MipsAsmPrinter is supposed to be delivering the ABI information for that path and it looks like it's still alive since AsmPrinter::doFinalization() is in the call stack at the point of the bad read. However, valgrind is clearly showing the invalid read occurred in memory belonging to a dead MipsAsmParser.

The MipsAsmParser presumably only exists because it's needed to parse the asm statement. My guess is that what's happening is MipsAsmPrinter delivers the ABI information as it's supposed to, then MipsAsmParser is constructed and substitutes its own MipsABIInfo pointer. MipsAsmParser is then destroyed, leaving a dangling pointer. Meanwhile, the pointer to MipsAsmPrinter's MipsABIInfo remains valid.

The target triples project will be able to solve this nicely since we'll be able to consult the longer-lived TargetTuple but we shouldn't make this fix depend on that. This patch seems reasonable in the mean time. LGTM

lib/Target/Mips/MipsTargetStreamer.h

 //===-- MipsTargetStreamer.h - Mips Target Streamer ------------*- C++ -*--===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_MIPS_MIPSTARGETSTREAMER_H
 #define LLVM_LIB_TARGET_MIPS_MIPSTARGETSTREAMER_H
 
 #include "MCTargetDesc/MipsABIFlagsSection.h"
 #include "MCTargetDesc/MipsABIInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/MC/MCELFStreamer.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 
 namespace llvm {
 
 struct MipsABIFlagsSection;
 
public:
   void forbidModuleDirective() { ModuleDirectiveAllowed = false; }
   void reallowModuleDirective() { ModuleDirectiveAllowed = true; }
   bool isModuleDirectiveAllowed() { return ModuleDirectiveAllowed; }
 
   // This method enables template classes to set internal abi flags
   // structure values.
   template <class PredicateLibrary>
   void updateABIInfo(const PredicateLibrary &P) {
-    ABI = &P.getABI();
+    ABI = P.getABI();
     ABIFlagsSection.setAllFromPredicates(P);
   }
 
   MipsABIFlagsSection &getABIFlagsSection() { return ABIFlagsSection; }
   const MipsABIInfo &getABI() const {
-    assert(ABI && "ABI hasn't been set!");
+    assert(ABI.hasValue() && "ABI hasn't been set!");
     return *ABI;
   }
 
 protected:
-  const MipsABIInfo *ABI;
+  llvm::Optional<MipsABIInfo> ABI;
   MipsABIFlagsSection ABIFlagsSection;
 
   bool GPRInfoSet;
   unsigned GPRBitMask;
   int GPROffset;
 
   bool FPRInfoSet;
   unsigned FPRBitMask;