[llvm] 236228f - [BitcodeReader] Replace unsupported constexprs in metadata with undef

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 5 05:38:34 PDT 2023


Author: Nikita Popov
Date: 2023-10-05T14:38:25+02:00
New Revision: 236228f43d018ebf11697610fe6504c167ed6ac8

URL: https://github.com/llvm/llvm-project/commit/236228f43d018ebf11697610fe6504c167ed6ac8
DIFF: https://github.com/llvm/llvm-project/commit/236228f43d018ebf11697610fe6504c167ed6ac8.diff

LOG: [BitcodeReader] Replace unsupported constexprs in metadata with undef

Metadata (via ValueAsMetadata) can reference constant expressions
that may no longer be supported. These references can both be in
function-local metadata and module metadata, if the same expression
is used in multiple functions. At least in theory, such references
could also be in metadata proper, rather than just inside
ValueAsMetadata references in calls.

Instead of trying to expand these expressions (which we can't
reliably do), pretend that the constant has been deleted, which
means that ValueAsMetadata references will get replaced with
undef metadata.

Fixes https://github.com/llvm/llvm-project/issues/68281.

Added: 
    llvm/test/Bitcode/Inputs/constexpr-to-instr-metadata-2.bc
    llvm/test/Bitcode/constexpr-to-instr-metadata-2.ll

Modified: 
    llvm/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/lib/Bitcode/Reader/MetadataLoader.cpp
    llvm/lib/Bitcode/Reader/MetadataLoader.h
    llvm/test/Bitcode/constexpr-to-instr-metadata.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index e56291859022eec..1d1ec988a93d847 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -4683,7 +4683,7 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
       case bitc::METADATA_BLOCK_ID:
         assert(DeferredMetadataInfo.empty() &&
                "Must read all module-level metadata before function-level");
-        if (Error Err = MDLoader->parseFunctionMetadata(CurBB))
+        if (Error Err = MDLoader->parseFunctionMetadata())
           return Err;
         break;
       case bitc::USELIST_BLOCK_ID:

diff  --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 74ccb2dd8f3261a..1e9ed5fcaa581b3 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -473,8 +473,7 @@ class MetadataLoader::MetadataLoaderImpl {
 
   Error parseOneMetadata(SmallVectorImpl<uint64_t> &Record, unsigned Code,
                          PlaceholderQueue &Placeholders, StringRef Blob,
-                         unsigned &NextMetadataNo,
-                         BasicBlock *ConstExprInsertBB);
+                         unsigned &NextMetadataNo);
   Error parseMetadataStrings(ArrayRef<uint64_t> Record, StringRef Blob,
                              function_ref<void(StringRef)> CallBack);
   Error parseGlobalObjectAttachment(GlobalObject &GO,
@@ -723,7 +722,7 @@ class MetadataLoader::MetadataLoaderImpl {
         TheModule(TheModule), Callbacks(std::move(Callbacks)),
         IsImporting(IsImporting) {}
 
-  Error parseMetadata(bool ModuleLevel, BasicBlock *ConstExprInsertBB);
+  Error parseMetadata(bool ModuleLevel);
 
   bool hasFwdRefs() const { return MetadataList.hasFwdRefs(); }
 
@@ -1048,8 +1047,7 @@ void MetadataLoader::MetadataLoaderImpl::callMDTypeCallback(Metadata **Val,
 
 /// Parse a METADATA_BLOCK. If ModuleLevel is true then we are parsing
 /// module level metadata.
-Error MetadataLoader::MetadataLoaderImpl::parseMetadata(
-    bool ModuleLevel, BasicBlock *ConstExprInsertBB) {
+Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
   if (!ModuleLevel && MetadataList.hasFwdRefs())
     return error("Invalid metadata: fwd refs into function blocks");
 
@@ -1132,7 +1130,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(
     if (Expected<unsigned> MaybeCode =
             Stream.readRecord(Entry.ID, Record, &Blob)) {
       if (Error Err = parseOneMetadata(Record, MaybeCode.get(), Placeholders,
-                                       Blob, NextMetadataNo, ConstExprInsertBB))
+                                       Blob, NextMetadataNo))
         return Err;
     } else
       return MaybeCode.takeError();
@@ -1173,8 +1171,7 @@ void MetadataLoader::MetadataLoaderImpl::lazyLoadOneMetadata(
   if (Expected<unsigned> MaybeCode =
           IndexCursor.readRecord(Entry.ID, Record, &Blob)) {
     if (Error Err =
-            parseOneMetadata(Record, MaybeCode.get(), Placeholders, Blob, ID,
-                             /* ConstExprInsertBB */ nullptr))
+            parseOneMetadata(Record, MaybeCode.get(), Placeholders, Blob, ID))
       report_fatal_error("Can't lazyload MD, parseOneMetadata: " +
                          Twine(toString(std::move(Err))));
   } else
@@ -1216,10 +1213,29 @@ void MetadataLoader::MetadataLoaderImpl::resolveForwardRefsAndPlaceholders(
   Placeholders.flush(MetadataList);
 }
 
+static Value *getValueFwdRef(BitcodeReaderValueList &ValueList, unsigned Idx,
+                             Type *Ty, unsigned TyID) {
+  Value *V = ValueList.getValueFwdRef(Idx, Ty, TyID,
+                                      /*ConstExprInsertBB*/ nullptr);
+  if (V)
+    return V;
+
+  // This is a reference to a no longer supported constant expression.
+  // Pretend that the constant was deleted, which will replace metadata
+  // references with undef.
+  // TODO: This is a rather indirect check. It would be more elegant to use
+  // a separate ErrorInfo for constant materialization failure and thread
+  // the error reporting through getValueFwdRef().
+  if (Idx < ValueList.size() && ValueList[Idx] &&
+      ValueList[Idx]->getType() == Ty)
+    return UndefValue::get(Ty);
+
+  return nullptr;
+}
+
 Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     SmallVectorImpl<uint64_t> &Record, unsigned Code,
-    PlaceholderQueue &Placeholders, StringRef Blob, unsigned &NextMetadataNo,
-    BasicBlock *ConstExprInsertBB) {
+    PlaceholderQueue &Placeholders, StringRef Blob, unsigned &NextMetadataNo) {
 
   bool IsDistinct = false;
   auto getMD = [&](unsigned ID) -> Metadata * {
@@ -1348,8 +1364,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
       if (Ty->isMetadataTy())
         Elts.push_back(getMD(Record[i + 1]));
       else if (!Ty->isVoidTy()) {
-        Value *V = ValueList.getValueFwdRef(Record[i + 1], Ty, TyID,
-                                            /*ConstExprInsertBB*/ nullptr);
+        Value *V = getValueFwdRef(ValueList, Record[i + 1], Ty, TyID);
         if (!V)
           return error("Invalid value reference from old metadata");
         Metadata *MD = ValueAsMetadata::get(V);
@@ -1373,7 +1388,7 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     if (!Ty || Ty->isMetadataTy() || Ty->isVoidTy())
       return error("Invalid record");
 
-    Value *V = ValueList.getValueFwdRef(Record[1], Ty, TyID, ConstExprInsertBB);
+    Value *V = getValueFwdRef(ValueList, Record[1], Ty, TyID);
     if (!V)
       return error("Invalid value reference from metadata");
 
@@ -2462,9 +2477,8 @@ MetadataLoader::MetadataLoader(BitstreamCursor &Stream, Module &TheModule,
     : Pimpl(std::make_unique<MetadataLoaderImpl>(
           Stream, TheModule, ValueList, std::move(Callbacks), IsImporting)) {}
 
-Error MetadataLoader::parseMetadata(bool ModuleLevel,
-                                    BasicBlock *ConstExprInsertBB) {
-  return Pimpl->parseMetadata(ModuleLevel, ConstExprInsertBB);
+Error MetadataLoader::parseMetadata(bool ModuleLevel) {
+  return Pimpl->parseMetadata(ModuleLevel);
 }
 
 bool MetadataLoader::hasFwdRefs() const { return Pimpl->hasFwdRefs(); }

diff  --git a/llvm/lib/Bitcode/Reader/MetadataLoader.h b/llvm/lib/Bitcode/Reader/MetadataLoader.h
index 3bbe51b2f70b25d..bab855ca6359107 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.h
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.h
@@ -48,8 +48,7 @@ struct MetadataLoaderCallbacks {
 class MetadataLoader {
   class MetadataLoaderImpl;
   std::unique_ptr<MetadataLoaderImpl> Pimpl;
-  Error parseMetadata(bool ModuleLevel,
-                      BasicBlock *ConstExprInsertBB = nullptr);
+  Error parseMetadata(bool ModuleLevel);
 
 public:
   ~MetadataLoader();
@@ -63,9 +62,7 @@ class MetadataLoader {
   Error parseModuleMetadata() { return parseMetadata(true); }
 
   // Parse a function metadata block
-  Error parseFunctionMetadata(BasicBlock *ConstExprInsertBB) {
-    return parseMetadata(false, ConstExprInsertBB);
-  }
+  Error parseFunctionMetadata() { return parseMetadata(false); }
 
   /// Set the mode to strip TBAA metadata on load.
   void setStripTBAA(bool StripTBAA = true);

diff  --git a/llvm/test/Bitcode/Inputs/constexpr-to-instr-metadata-2.bc b/llvm/test/Bitcode/Inputs/constexpr-to-instr-metadata-2.bc
new file mode 100644
index 000000000000000..732e60110cfc4b4
Binary files /dev/null and b/llvm/test/Bitcode/Inputs/constexpr-to-instr-metadata-2.bc 
diff er

diff  --git a/llvm/test/Bitcode/constexpr-to-instr-metadata-2.ll b/llvm/test/Bitcode/constexpr-to-instr-metadata-2.ll
new file mode 100644
index 000000000000000..24938989e21b151
--- /dev/null
+++ b/llvm/test/Bitcode/constexpr-to-instr-metadata-2.ll
@@ -0,0 +1,7 @@
+; RUN: llvm-dis -expand-constant-exprs < %S/Inputs/constexpr-to-instr-metadata-2.bc | FileCheck %s
+
+; CHECK-LABEL: define void @_ZN4alsa3pcm3PCM17hw_params_current17hf1c237aece2f69c4E() {
+; CHECK: call void @llvm.dbg.value(metadata ptr undef, metadata !4, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !14
+
+; CHECK-LABEL: define void @_ZN4alsa3pcm8HwParams3any17h02a64cfc85ce8a66E() {
+; CHECK: call void @llvm.dbg.value(metadata ptr undef, metadata !23, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !28

diff  --git a/llvm/test/Bitcode/constexpr-to-instr-metadata.ll b/llvm/test/Bitcode/constexpr-to-instr-metadata.ll
index 78b5e30dd3154b5..39a1b2687ae86b5 100644
--- a/llvm/test/Bitcode/constexpr-to-instr-metadata.ll
+++ b/llvm/test/Bitcode/constexpr-to-instr-metadata.ll
@@ -1,12 +1,4 @@
 ; RUN: llvm-dis -expand-constant-exprs < %S/Inputs/constexpr-to-instr-metadata.bc | FileCheck %s
 
 ; CHECK-LABEL: define void @test() {
-; CHECK: %constexpr = ptrtoint ptr @g to i32
-; CHECK: %constexpr1 = zext i32 %constexpr to i64
-; CHECK: %constexpr2 = ptrtoint ptr @g to i64
-; CHECK: %constexpr3 = lshr i64 %constexpr2, 32
-; CHECK: %constexpr4 = trunc i64 %constexpr3 to i32
-; CHECK: %constexpr5 = zext i32 %constexpr4 to i64
-; CHECK: %constexpr6 = shl i64 %constexpr5, 32
-; CHECK: %constexpr7 = or i64 %constexpr1, %constexpr6
-; CHECK: call void @llvm.dbg.value(metadata i64 %constexpr7, metadata !4, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !13
+; CHECK: call void @llvm.dbg.value(metadata i64 undef, metadata !4, metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)), !dbg !13


        


More information about the llvm-commits mailing list