[OpaquePtr][LLParser] Automatically detect opaque pointers in .ll files

[aeubanks]

Summary

This allows us to not have to specify -opaque-pointers when updating
IR tests from typed pointers to opaque pointers.

We detect opaque pointers in .ll files by looking for relevant tokens,
either "ptr" or "*".

[aeubanks]

update tests

[aeubanks]

new test

[nikic]

I'm not really convinced by this. I think automatically going into opaque pointer mode if ptr is mentioned in .ll makes some sense, but I'm not a fan on determining the mode based on first constructed pointer. For bitcode, we can embed a marker for opaque pointer mode rather than guessing.

llvm/include/llvm/IR/LLVMContext.h

310

Why drop this? This is needed for per-frontend enablement, which we'll still need, right?

[aeubanks]

update

[aeubanks]

llvm/include/llvm/IR/LLVMContext.h

310

I was under the impression that frontends would just pass -mllvm -opaque-pointers (or whatever the frontend equivalent), since it doesn't seem necessary to support multiple LLVMContexts with different opaque pointers flags. But either way I've restored it in this change.

[nikic]

LGTM

[hctim]

llvm/lib/IR/LLVMContextImpl.cpp

51

Hi, looks like this broke the ASan buildbot.

Help reproducing the bot can be found here. The init-order-fiasco bug is copied below for your convenience.

--
********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..
FAIL: LLVM :: tools/llvm-stress/help.test (73153 of 82954)
******************** TEST 'LLVM :: tools/llvm-stress/help.test' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-stress --help | /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/tools/llvm-stress/help.test --check-prefix HELP --implicit-check-not='{{[Oo]}}ptions:'
--
Exit Code: 2
Command Output (stderr):
--
=================================================================
==53409==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x0000013b96a8 at pc 0x000000867d43 bp 0x7ffeb2c63cb0 sp 0x7ffeb2c63ca8
READ of size 2 at 0x0000013b96a8 thread T0
    #0 0x867d42 in getNumOccurrences /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/Support/CommandLine.h:395:49
    #1 0x867d42 in llvm::LLVMContextImpl::LLVMContextImpl(llvm::LLVMContext&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/LLVMContextImpl.cpp:51:24
    #2 0x85171e in llvm::LLVMContext::LLVMContext() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/LLVMContext.cpp:34:40
    #3 0x6c73bd in __cxx_global_var_init.12 /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-stress/llvm-stress.cpp:72:20
    #4 0x6c73bd in _GLOBAL__sub_I_llvm_stress.cpp /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llvm-stress/llvm-stress.cpp
    #5 0xc025c4 in __libc_csu_init (/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-stress+0xc025c4)
    #6 0x7f634e91d029 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24029) (BuildId: 18b9a9a8c523e5cfe5b5d946d605d09242f09798)
    #7 0x5d8079 in _start (/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-stress+0x5d8079)
0x0000013b96a8 is located 8 bytes inside of global variable 'OpaquePointersCL' defined in '/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/IR/LLVMContextImpl.cpp:38:5' (0x13b96a0) of size 224
  registered at:
    #0 0x5efcb0 in __asan_register_globals.part.19 /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:359:3
    #1 0x88520e in asan.module_ctor LLVMContextImpl.cpp
    #2 0xc025c4 in __libc_csu_init (/b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llvm-stress+0xc025c4)
SUMMARY: AddressSanitizer: initialization-order-fiasco /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/include/llvm/Support/CommandLine.h:395:49 in getNumOccurrences
Shadow bytes around the buggy address:
  0x00008026f280: f6 f6 f6 f6 00 f9 f9 f9 00 f9 f9 f9 01 f9 f9 f9
  0x00008026f290: 01 f9 f9 f9 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f2a0: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f2b0: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f2c0: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
=>0x00008026f2d0: f6 f6 f6 f6 f6[f6]f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f2e0: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f2f0: f6 f6 f6 f6 00 00 f9 f9 00 00 f9 f9 00 00 f9 f9
  0x00008026f300: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f310: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
  0x00008026f320: f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6 f6
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==53409==ABORTING
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/tools/llvm-stress/help.test --check-prefix HELP --implicit-check-not={{[Oo]}}ptions:
--

[aeubanks]

Those were already fixed at head with 3210647e5b687c49d363e271fe072c213cd9cc2d, 7678e8ebbbc90d227912638fb7f596febfe75bf6, and 9276fed284f6a8fdbb3f4f56eee03b256e8172e1

llvm/include/llvm/AsmParser/LLLexer.h

class LLLexer {
     SourceMgr &SM;
     LLVMContext &Context;
 
     // Information about the current token.
     const char *TokStart;
     lltok::Kind CurKind;
     std::string StrVal;
     unsigned UIntVal;
-    Type *TyVal;
+    Type *TyVal = nullptr;
     APFloat APFloatVal;
     APSInt  APSIntVal;
 
     // When false (default), an identifier ending in ':' is a label token.
     // When true, the ':' is treated as a separate token.
     bool IgnoreColonInIdentifiers;
 
   public:

llvm/include/llvm/AsmParser/LLParser.h

struct ValID {
     }
   };
 
   class LLParser {
   public:
     typedef LLLexer::LocTy LocTy;
   private:
     LLVMContext &Context;
+    // Lexer to determine whether to use opaque pointers or not.
+    LLLexer OPLex;
     LLLexer Lex;
     // Module being parsed, null if we are only parsing summary index.
     Module *M;
     // Summary index being parsed, null if we are only parsing Module.
     ModuleSummaryIndex *Index;
     SlotMapping *Slots;
 
     SmallVector<Instruction*, 64> InstsWithTBAATag;
private:
     bool UpgradeDebugInfo;
 
     std::string SourceFileName;
 
   public:
     LLParser(StringRef F, SourceMgr &SM, SMDiagnostic &Err, Module *M,
              ModuleSummaryIndex *Index, LLVMContext &Context,
              SlotMapping *Slots = nullptr)
-        : Context(Context), Lex(F, SM, Err, Context), M(M), Index(Index),
-          Slots(Slots), BlockAddressPFS(nullptr) {}
+        : Context(Context), OPLex(F, SM, Err, Context),
+          Lex(F, SM, Err, Context), M(M), Index(Index), Slots(Slots),
+          BlockAddressPFS(nullptr) {}
     bool Run(
         bool UpgradeDebugInfo, DataLayoutCallbackTy DataLayoutCallback =
                                    [](StringRef) { return None; });
 
     bool parseStandaloneConstantValue(Constant *&C, const SlotMapping *Slots);
 
     bool parseTypeAtBeginning(Type *&Ty, unsigned &Read,
                               const SlotMapping *Slots);

llvm/include/llvm/IR/LLVMContext.h

#undef LLVM_FIXED_MD_KIND
 
   /// Set the object which can disable optional passes and individual
   /// optimizations at compile time.
   ///
   /// The lifetime of the object must be guaranteed to extend as long as the
   /// LLVMContext is used by compilation.
   void setOptPassGate(OptPassGate&);
 
+  /// Whether we've decided on using opaque pointers or typed pointers yet.
+  bool hasSetOpaquePointersValue() const;
+
   /// Enable opaque pointers. Can only be called before creating the first
   /// pointer type.
   void enableOpaquePointers() const;
 
   /// Whether typed pointers are supported. If false, all pointers are opaque.
   bool supportsTypedPointers() const;
 
 private:
   // Module needs access to the add/removeModule methods.
   friend class Module;
 

llvm/lib/AsmParser/LLLexer.cpp

#define TYPEKEYWORD(STR, LLVMTY)                                               \
   TYPEKEYWORD("ppc_fp128", Type::getPPC_FP128Ty(Context));
   TYPEKEYWORD("label",     Type::getLabelTy(Context));
   TYPEKEYWORD("metadata",  Type::getMetadataTy(Context));
   TYPEKEYWORD("x86_mmx",   Type::getX86_MMXTy(Context));
   TYPEKEYWORD("x86_amx",   Type::getX86_AMXTy(Context));
   TYPEKEYWORD("token",     Type::getTokenTy(Context));
 
   if (Keyword == "ptr") {
-    if (Context.supportsTypedPointers()) {
+    // enableOpaquePointers() must be called before creating any pointer types.
+    if (!Context.hasSetOpaquePointersValue()) {
+      Context.enableOpaquePointers();
+    } else if (Context.supportsTypedPointers()) {
       Warning("ptr type is only supported in -opaque-pointers mode");
       return lltok::Error;
     }
     TyVal = PointerType::getUnqual(Context);
     return lltok::Type;
   }
 
 #undef TYPEKEYWORD

llvm/lib/AsmParser/LLParser.cpp

 
 static std::string getTypeString(Type *T) {
   std::string Result;
   raw_string_ostream Tmp(Result);
   Tmp << *T;
   return Tmp.str();
 }
 
+static void setContextOpaquePointers(LLLexer &L, LLVMContext &C) {
+  while (true) {
+    lltok::Kind K = L.Lex();
+    // LLLexer will set the opaque pointers option in LLVMContext if it sees an
+    // explicit "ptr".
+    if (K == lltok::star || K == lltok::Error || K == lltok::Eof ||
+        isa_and_nonnull<PointerType>(L.getTyVal())) {
+      return;
+    }
+  }
+}
+
 /// Run: module ::= toplevelentity*
 bool LLParser::Run(bool UpgradeDebugInfo,
                    DataLayoutCallbackTy DataLayoutCallback) {
+  // If we haven't decided on whether or not we're using opaque pointers, do a
+  // quick lex over the tokens to see if we explicitly construct any typed or
+  // opaque pointer types.
+  // Don't bail out on an error so we do the same work in the parsing below
+  // regardless of if --opaque-pointers is set.
+  if (!Context.hasSetOpaquePointersValue())
+    setContextOpaquePointers(OPLex, Context);
+
   // Prime the lexer.
   Lex.Lex();
 
   if (Context.shouldDiscardValueNames())
     return error(
         Lex.getLoc(),
         "Can't read textual IR with a Context that discards named Values");
 

llvm/lib/IR/LLVMContext.cpp

 const DiagnosticHandler *LLVMContext::getDiagHandlerPtr() const {
   return pImpl->DiagHandler.get();
 }
 
 std::unique_ptr<DiagnosticHandler> LLVMContext::getDiagnosticHandler() {
   return std::move(pImpl->DiagHandler);
 }
 
+bool LLVMContext::hasSetOpaquePointersValue() const {
+  return pImpl->hasOpaquePointersValue();
+}
+
 void LLVMContext::enableOpaquePointers() const {
   assert(pImpl->PointerTypes.empty() && pImpl->ASPointerTypes.empty() &&
          "Must be called before creating any pointer types");
   pImpl->setOpaquePointers(true);
 }
 
 bool LLVMContext::supportsTypedPointers() const {
   return !pImpl->getOpaquePointers();
 }

llvm/lib/IR/LLVMContextImpl.h

#include "llvm/IR/Metadata.def"
   ///
   /// The lifetime of the object must be guaranteed to extend as long as the
   /// LLVMContext is used by compilation.
   void setOptPassGate(OptPassGate &);
 
   // TODO: clean up the following after we no longer support non-opaque pointer
   // types.
   bool getOpaquePointers();
+  bool hasOpaquePointersValue();
   void setOpaquePointers(bool OP);
 
 private:
   Optional<bool> OpaquePointers;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_IR_LLVMCONTEXTIMPL_H

llvm/lib/IR/LLVMContextImpl.cpp

LLVMContextImpl::LLVMContextImpl(LLVMContext &C)
     : DiagHandler(std::make_unique<DiagnosticHandler>()),
       VoidTy(C, Type::VoidTyID), LabelTy(C, Type::LabelTyID),
       HalfTy(C, Type::HalfTyID), BFloatTy(C, Type::BFloatTyID),
       FloatTy(C, Type::FloatTyID), DoubleTy(C, Type::DoubleTyID),
       MetadataTy(C, Type::MetadataTyID), TokenTy(C, Type::TokenTyID),
       X86_FP80Ty(C, Type::X86_FP80TyID), FP128Ty(C, Type::FP128TyID),
       PPC_FP128Ty(C, Type::PPC_FP128TyID), X86_MMXTy(C, Type::X86_MMXTyID),
       X86_AMXTy(C, Type::X86_AMXTyID), Int1Ty(C, 1), Int8Ty(C, 8),
-      Int16Ty(C, 16), Int32Ty(C, 32), Int64Ty(C, 64), Int128Ty(C, 128) {}
+      Int16Ty(C, 16), Int32Ty(C, 32), Int64Ty(C, 64), Int128Ty(C, 128) {
+  if (OpaquePointersCL.getNumOccurrences()) {
+    OpaquePointers = OpaquePointersCL;
+  }
+}
 
 LLVMContextImpl::~LLVMContextImpl() {
   // NOTE: We need to delete the contents of OwnedModules, but Module's dtor
   // will call LLVMContextImpl::removeModule, thus invalidating iterators into
   // the container. Avoid iterators during this operation:
   while (!OwnedModules.empty())
     delete *OwnedModules.begin();
 
if (!OPG)
     OPG = &(*OptBisector);
   return *OPG;
 }
 
 void LLVMContextImpl::setOptPassGate(OptPassGate& OPG) {
   this->OPG = &OPG;
 }
 
+bool LLVMContextImpl::hasOpaquePointersValue() {
+  return OpaquePointers.hasValue();
+}
+
 bool LLVMContextImpl::getOpaquePointers() {
   if (LLVM_UNLIKELY(!(OpaquePointers.hasValue())))
-    OpaquePointers = OpaquePointersCL;
+    OpaquePointers = false;
   return *OpaquePointers;
 }
 
 void LLVMContextImpl::setOpaquePointers(bool OP) { OpaquePointers = OP; }

llvm/test/Assembler/ptr-outside-opaque-pointers-mode.ll

-; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+; RUN: not llvm-as < %s -disable-output --opaque-pointers=0 2>&1 | FileCheck %s
 
 ; CHECK: warning: ptr type is only supported in -opaque-pointers mode
 ; CHECK: error: expected type
 define void @f(ptr %a) {
     ret void
 }

llvm/test/Bitcode/opaque-ptr.ll

-; RUN: llvm-as --opaque-pointers < %s | not llvm-dis 2>&1 | FileCheck %s
+; RUN: llvm-as --opaque-pointers < %s | not llvm-dis --opaque-pointers=0 2>&1 | FileCheck %s
 
 ; CHECK: error: Opaque pointers are only supported in -opaque-pointers mode
 
 @g = external global i16
 
 define void @f(i32* %p) {
   %a = alloca i17
   ret void
 }

llvm/test/Other/mixed-opaque-ptrs-2.ll

+; RUN: llvm-as -disable-output %s 2>&1
+; FIXME: this should err out saying not to mix `ptr` and `foo*`
+define void @f(ptr) {
+  %a = alloca i32*
+  ret void
+}

llvm/test/Other/mixed-opaque-ptrs.ll

+; RUN: not llvm-as -disable-output %s 2>&1 | FileCheck %s
+; CHECK: ptr type is only supported in -opaque-pointers mode
+define void @f(i32*) {
+  %a = alloca ptr
+  ret void
+}