[Bitcode] Include indirect users of BlockAddresses in bitcode

[twd2]

Summary

The original fix (commit 23ec5782c3cc) of https://github.com/llvm/llvm-project/issues/52787 only adds Functions that have Instructions that directly use BlockAddresses into the bitcode (FUNC_CODE_BLOCKADDR_USERS).

However, in either @rickyz's original reproducing code:

void f(long);

__attribute__((noinline)) static void fun(long x) {
  f(x + 1);
}

void repro(void) {
  fun(({
    label:
      (long)&&label;
  }));
}

...
define dso_local void @repro() #0 {
entry:
  br label %label

label:                                            ; preds = %entry
  tail call fastcc void @fun()
  ret void
}

define internal fastcc void @fun() unnamed_addr #1 {
entry:
  tail call void @f(i64 add (i64 ptrtoint (i8* blockaddress(@repro, %label) to i64), i64 1)) #3
  ret void
}
...

or the xfs and overlayfs in the Linux kernel, BlockAddresses (e.g., i8* blockaddress(@repro, %label)) may first compose ConstantExprs (e.g., i64 ptrtoint (i8* blockaddress(@repro, %label) to i64)) and then used by Instructions. This case is not handled by the original fix.

This patch adds *indirect* users of BlockAddresses, i.e., the Instructions using some Constants which further use the BlockAddresses, into the bitcode as well, by doing depth-first searches.

Fixes: https://github.com/llvm/llvm-project/issues/52787
Fixes: 23ec5782c3cc ("[Bitcode] materialize Functions early when BlockAddress taken")

[twd2]

FYI, before the patch:

$ cat repro.c
void f(long);

__attribute__((noinline)) static void fun(long x) {
  f(x + 1);
}

void repro(void) {
  fun(({
    label:
      (long)&&label;
  }));
}
$ clang -O2 -flto -c repro.c -o repro.i
$ llvm-bcanalyzer -dump repro.i | grep USER                         
$ ld.lld repro.i
ld.lld: error: Never resolved function from blockaddress (Producer: 'LLVM15.0.0git' Reader: 'LLVM 15.0.0git')

After the patch:

$ clang -O2 -flto -c repro.c -o repro.i
$ llvm-bcanalyzer -dump repro.i | grep USER                         
    <BLOCKADDR_USERS op0=1/>
                      1        28                    BLOCKADDR_USERS
$ ld.lld repro.i
ld.lld: warning: cannot find entry symbol _start; not setting start address

[nickdesaulniers]

Thanks for the patch! Indeed, the users _might_ be Constants rather than just Instructions. One thing to think about is that you can declare global Constants. I used the dyn_cast<Instruction> to check that this was not the case. I think you'll need to differentiate that, too. Meaning, if the user is a Constant, are the Constant's users Instructions (or not if simply GlobalVar).

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3405–3408

Can we simply add additional dyn_cast here for ConstantData, ConstantAggregate, and ConstantExpr rather than two new data structures and loops?

That said, I guess Constant's are unique to a module, so not revisiting them is nice.

3416–3417

Consider adding test coverage for 3 different types of Users.

[twd2]

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3416–3417

Thanks for the comment!

I just find that ConstantDatas take no operand, so U will never be a ConstantData. I'll remove this condition and add tests for the remaining 2 types.

[twd2]

Remove the ConstantData condition and add tests for the remaining 2 types. This update also adds a test for nested users of BlockAddresses.

[nickdesaulniers]

Please add a test for:

@foo = global i8* blockaddress(@repro, %label)

define void @repro() {
  br label %label

label:
  ret void
}

That this DOES NOT emit a BLOCKADDR_USERS. This is the main case I care about, and I think we can be do less work with the queue in such cases. Differentiating when the use is an Instruction or not is important, because we only need to emit BLOCKADDR_USERS for Instructions, not globals.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3405–3422

Can we use a SmallVector (push_back, pop_back_val) instead?

[twd2]

In D124878#3489625, @nickdesaulniers wrote:

Thanks for the patch! Indeed, the users _might_ be Constants rather than just Instructions. One thing to think about is that you can declare global Constants. I used the dyn_cast<Instruction> to check that this was not the case. I think you'll need to differentiate that, too. Meaning, if the user is a Constant, are the Constant's users Instructions (or not if simply GlobalVar).

The users of Constants are not only Instructions and GlobalValues, but also other Constants.
For example, in rickyz's original reproducing code, the only user of i8* blockaddress(@repro, %label) is a ConstantExpr, i64 ptrtoint (i8* blockaddress(@repro, %label) to i64).
This ConstantExpr will not pass the dyn_cast<Instruction> check, and no Instruction will be encoded in the bitcode.
Further, the only user of i64 ptrtoint (... to i64) is also a ConstantExpr, i64 add (i64 ptrtoint (i8* blockaddress(@repro, %label) to i64), i64 1).
Finally, the only user of i64 add (..., i64 1) is the tail call ... Instruction in Function @fun.
We should find this Instruction and add the Function into the bitcode.

So, we might have to do a searching loop to first identify what Constants (directly or recursively) use the given BlockAddress, and what Instructions (Functions) use these Constants, as my patch proposes.

[twd2]

Add the GlobalValue test mentioned by @nickdesaulniers

[twd2]

In D124878#3492365, @twd2 wrote:

In D124878#3489625, @nickdesaulniers wrote:

Thanks for the patch! Indeed, the users _might_ be Constants rather than just Instructions. One thing to think about is that you can declare global Constants. I used the dyn_cast<Instruction> to check that this was not the case. I think you'll need to differentiate that, too. Meaning, if the user is a Constant, are the Constant's users Instructions (or not if simply GlobalVar).

The users of Constants are not only Instructions and GlobalValues, but also other Constants.
For example, in rickyz's original reproducing code, the only user of i8* blockaddress(@repro, %label) is a ConstantExpr, i64 ptrtoint (i8* blockaddress(@repro, %label) to i64).
This ConstantExpr will not pass the dyn_cast<Instruction> check, and no Instruction will be encoded in the bitcode.
Further, the only user of i64 ptrtoint (... to i64) is also a ConstantExpr, i64 add (i64 ptrtoint (i8* blockaddress(@repro, %label) to i64), i64 1).
Finally, the only user of i64 add (..., i64 1) is the tail call ... Instruction in Function @fun.
We should find this Instruction and add the Function into the bitcode.

So, we might have to do a searching loop to first identify what Constants (directly or recursively) use the given BlockAddress, and what Instructions (Functions) use these Constants, as my patch proposes.

This may visit some Constants that are not used by Instructions (e.g., used by GlobalValues) at the end, but I'm afraid that we have to visit them before getting the ultimate users.

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

3405–3422

Sure, I believe it's better!

[twd2]

Use SmallVector instead of deque

[dexonsmith]

At a high-level, the design for writing bitcode is a two-pass traversal algorithm:

• First pass in ValueEnumerator indexes everything
• Second pass in BitcodeWriter writes things

It feels like this new logic could be put into the first pass in ValueEnumerator. (Probably the previous patch, which this builds on, should have done that as well, to avoid iterating over users() in BitcodeWriter, but I guess I didn't notice!)

I think you add some bookkeeping to ValueEnumerator::EnumerateValue(). It currently has:

if (const Constant *C = dyn_cast<Constant>(V)) {
  if (isa<GlobalValue>(C)) {
    // Initializers for globals are handled explicitly elsewhere.
  } else if (C->getNumOperands()) {
    for (User::const_op_iterator I = C->op_begin(), E = C->op_end();
         I != E; ++I)
      if (!isa<BasicBlock>(*I)) // Don't enumerate BB operand to BlockAddress.
        EnumerateValue(*I);

you could change this to something like:

if (const Constant *C = dyn_cast<Constant>(V)) {
  if (isa<GlobalValue>(C)) {
    // Initializers for globals are handled explicitly elsewhere.
  } else if (C->getNumOperands()) {
    SmallSetVector<const BasicBlock *, 4> FoundBBs;
    for (User::const_op_iterator I = C->op_begin(), E = C->op_end();
         I != E; ++I) {
      // Don't enumerate BB operand to BlockAddress, but do track them.
      if (const BasicBlock *BB = dyn_cast<BasicBlock>(&*I)) {
        FoundBBs.insert(BB);
        continue;
      }
      EnumerateValue(*I);
      auto I = BlockAddressesForConstant.find(&cast<Constant>(*I));
      if (I != BlockAddressesForConstant())
        FoundBBs.insert(I->begin(), I->end());
    }
    if (!BlockAddresses.empty())
      BlockAddressesForConstant[C].assign(FoundBBs.begin(), FoundBBs.end());

where ValueEnumerator has a new thing:

SmallDenseMap<const Constant *, SmallVector<const BasicBlock *, 0>> BlockAddressesForConstant;

Then you can change incorporateFunction, which currently has:

// Add all function-level constants to the value table.
for (const BasicBlock &BB : F) {
  for (const Instruction &I : BB) {
    for (const Use &OI : I.operands()) {
      if ((isa<Constant>(OI) && !isa<GlobalValue>(OI)) || isa<InlineAsm>(OI))
        EnumerateValue(OI);
    }
    if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I))
      EnumerateValue(SVI->getShuffleMaskForBitcode());
  }
  BasicBlocks.push_back(&BB);
  ValueMap[&BB] = BasicBlocks.size();
}

to be:

// Add all function-level constants to the value table.
SmallSetVector<const BasicBlock *> FoundBlockAddresses;
for (const BasicBlock &BB : F) {
  for (const Instruction &I : BB) {
    for (const Use &OI : I.operands()) {
      if (isa<GlobalValue>(OI))
        continue;
      auto *C = dyn_cast<Constant>(OI);
      if (!C && !isa<InlineAsm>(OI))
        continue;
      EnumerateValue(OI);
      if (!C)
        continue;
      auto BBs = BlockAddressesForConstant.find(C);
      if (BBs != BlockAddressesForConstant.end())
        FoundBlockAddresses.append(BBs->begin(), BBs->end());
    }
    if (auto *SVI = dyn_cast<ShuffleVectorInst>(&I))
      EnumerateValue(SVI->getShuffleMaskForBitcode());
  }
  BasicBlocks.push_back(&BB);
  ValueMap[&BB] = BasicBlocks.size();
}

if (!FoundBlockAddresses.empty()) {
  // Filter out ones that are local.
  SmallVector<const BasicBlock *> ForeignBlockAddresses;
  llvm::copy_if(FoundBlockAddresses, [&](const BasicBlock *BB) { return BB->getOwner() != &F; },
                std::back_inserter(ForeignBlockAddresses));
  if (!ForeignBlockAddresses.empty())
    FunctionBlockAddresses[&F] = std::move(ForeignBlockAddresses);
}

where ValueEnumerator has:

SmallDenseMap<const Function *, SmallVector<const BasicBlock *, 0>> FunctionBlockAddresses;

Then BitcodeWriter has no need to walk upwards through users(). It can just look at FunctionBlockAddresses.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

3095 ↗

(On Diff #427143)

Nit: please commit any unrelated whitespace changes in separate NFC patches.

[twd2]

In D124878#3492498, @dexonsmith wrote:

At a high-level, the design for writing bitcode is a two-pass traversal algorithm:

• First pass in ValueEnumerator indexes everything
• Second pass in BitcodeWriter writes things

It feels like this new logic could be put into the first pass in ValueEnumerator. (Probably the previous patch, which this builds on, should have done that as well, to avoid iterating over users() in BitcodeWriter, but I guess I didn't notice!)

I just tried your advice to update this patch (not uploaded yet) but found the existing logic is

for each function
    incorporateFunction, which enumerates function-local constants, including BlockAddresses, and we can maintain the users of referred foreign functions here
    writeFunction, which emits BLOCKADDR_USERS records and the function-local constants

However, for every function, before emitting BLOCKADDR_USERS records, we should expect that we have visited all the constants, including function-local ones, and thus we have collected all the users of the blockaddresses in the function.

Can we enumerate (and thus emit) all the constants at the beginning, or add a traversing loop dedicated to collecting the users of blockaddresses?
The latter will touch all the constants one more time.

[twd2]

drop unrelated whitespace changes

[dexonsmith]

In D124878#3495317, @twd2 wrote:

In D124878#3492498, @dexonsmith wrote:

At a high-level, the design for writing bitcode is a two-pass traversal algorithm:

• First pass in ValueEnumerator indexes everything
• Second pass in BitcodeWriter writes things

It feels like this new logic could be put into the first pass in ValueEnumerator. (Probably the previous patch, which this builds on, should have done that as well, to avoid iterating over users() in BitcodeWriter, but I guess I didn't notice!)

I just tried your advice to update this patch (not uploaded yet) but found the existing logic is

for each function
    incorporateFunction, which enumerates function-local constants, including BlockAddresses, and we can maintain the users of referred foreign functions here
    writeFunction, which emits BLOCKADDR_USERS records and the function-local constants

However, for every function, before emitting BLOCKADDR_USERS records, we should expect that we have visited all the constants, including function-local ones, and thus we have collected all the users of the blockaddresses in the function.

Can we enumerate (and thus emit) all the constants at the beginning, or add a traversing loop dedicated to collecting the users of blockaddresses?
The latter will touch all the constants one more time.

Ah, sorry for the misdirection; I'd forgotten that incorporate+write are in an inner loop. Enumerating all function-local constants ahead of time would have side effects on the bitcode output (effectively changing numbering to be global). Traversing all constants an extra time just to index blockaddress (without enumerating) would penalize serialization time in the common case, where there are no/few blockaddresses.

I suggest for now that you go ahead with "traverse users()" logic for fixing the bug (maybe it'd be better to move the logic to ValueEnumerator, but I'll leave that up to you).

(A more invasive change would be to index in ValueEnumerator as I suggested, and write a placeholder record in functions that export BlockAddress that can be patched up later somehow after indexing is complete (the patch could point into a new bitcode block). This would probably add more complexity than it'd be worth though...)

[twd2]

In D124878#3503982, @dexonsmith wrote:

(A more invasive change would be to index in ValueEnumerator as I suggested, and write a placeholder record in functions that export BlockAddress that can be patched up later somehow after indexing is complete (the patch could point into a new bitcode block). This would probably add more complexity than it'd be worth though...)

Thanks for the explanation. I just leave the current state here: https://reviews.llvm.org/differential/diff/428488/
Maybe we can do this later.

[twd2]

Thank you all for reviewing.

Could you please commit this patch on my behalf? Wende Tan <[e-mail]>

And, a small question: should we use SmallSetVector instead in SmallPtrSet<Function *, 4> BlockAddressUsers; to make sure that the order of users encoded in the bitcode is independent of the internal algorithm of SmallPtrSet?

[nickdesaulniers]

In D124878#3504571, @twd2 wrote:

Thank you all for reviewing.

You're welcome. Thanks for the patch!

Could you please commit this patch on my behalf? Wende Tan <[e-mail]>

Sure, I can take care of that.

And, a small question: should we use SmallSetVector instead in SmallPtrSet<Function *, 4> BlockAddressUsers; to make sure that the order of users encoded in the bitcode is independent of the internal algorithm of SmallPtrSet?

We don't iterate BlockAddressUsersVisited; I think the code is fine as is. Am I missing something?

[twd2]

In D124878#3504573, @nickdesaulniers wrote:

In D124878#3504571, @twd2 wrote:

Thank you all for reviewing.

You're welcome. Thanks for the patch!

Could you please commit this patch on my behalf? Wende Tan <[e-mail]>

Sure, I can take care of that.

And, a small question: should we use SmallSetVector instead in SmallPtrSet<Function *, 4> BlockAddressUsers; to make sure that the order of users encoded in the bitcode is independent of the internal algorithm of SmallPtrSet?

We don't iterate BlockAddressUsersVisited; I think the code is fine as is. Am I missing something?

It's BlockAddressUsers in Line 3368, your patch added it.

[nickdesaulniers]

In D124878#3504577, @twd2 wrote:

In D124878#3504573, @nickdesaulniers wrote:

In D124878#3504571, @twd2 wrote:

And, a small question: should we use SmallSetVector instead in SmallPtrSet<Function *, 4> BlockAddressUsers; to make sure that the order of users encoded in the bitcode is independent of the internal algorithm of SmallPtrSet?

We don't iterate BlockAddressUsersVisited; I think the code is fine as is. Am I missing something?

It's BlockAddressUsers in Line 3368, your patch added it.

Ah! Sorry, I mixed up which variable you were referring to. Yeah, looks like it should be. Care to fix that in this patch?

[twd2]

Use SmallSetVector instead in SmallPtrSet<Function *, 4> BlockAddressUsers; to make sure that the order of users encoded in the bitcode is independent of the internal algorithm of SmallPtrSet.

[twd2]

In D124878#3504669, @nickdesaulniers wrote:

Ah! Sorry, I mixed up which variable you were referring to. Yeah, looks like it should be. Care to fix that in this patch?

Yes, I just updated the diff. If everything is ok, I think we can commit this patch :)

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

 #include "ValueEnumerator.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Bitcode/BitcodeCommon.h"
 #include "llvm/Bitcode/BitcodeReader.h"
void ModuleBitcodeWriter::writeFunction(
   writeFunctionMetadata(F);
 
   // Keep a running idea of what the instruction ID is.
   unsigned InstID = CstEnd;
 
   bool NeedsMetadataAttachment = F.hasMetadata();
 
   DILocation *LastDL = nullptr;
-  SmallPtrSet<Function *, 4> BlockAddressUsers;
+  SmallSetVector<Function *, 4> BlockAddressUsers;
 
   // Finally, emit all the instructions, in order.
   for (const BasicBlock &BB : F) {
     for (const Instruction &I : BB) {
       writeInstruction(I, InstID, Vals);
 
       if (!I.getType()->isVoidTy())
         ++InstID;
for (const Instruction &I : BB) {
       Vals.push_back(DL->isImplicitCode());
       Stream.EmitRecord(bitc::FUNC_CODE_DEBUG_LOC, Vals);
       Vals.clear();
 
       LastDL = DL;
     }
 
     if (BlockAddress *BA = BlockAddress::lookup(&BB)) {
-      for (User *U : BA->users()) {
+      SmallVector<Value *, 16> BlockAddressUsersStack { BA };
+      SmallPtrSet<Value *, 16> BlockAddressUsersVisited { BA };
+
+      while (!BlockAddressUsersStack.empty()) {
+        Value *V = BlockAddressUsersStack.pop_back_val();
+
+        for (User *U : V->users()) {
+          if ((isa<ConstantAggregate>(U) || isa<ConstantExpr>(U)) &&
+              !BlockAddressUsersVisited.contains(U)) {
+            BlockAddressUsersStack.push_back(U);
+            BlockAddressUsersVisited.insert(U);
+          }
+
           if (auto *I = dyn_cast<Instruction>(U)) {
             Function *P = I->getParent()->getParent();
             if (P != &F)
               BlockAddressUsers.insert(P);
           }
         }
       }
     }
+  }
 
   if (!BlockAddressUsers.empty()) {
     SmallVector<uint64_t, 4> Record;
     Record.reserve(BlockAddressUsers.size());
     for (Function *F : BlockAddressUsers)
       Record.push_back(VE.getValueID(F));
     Stream.EmitRecord(bitc::FUNC_CODE_BLOCKADDR_USERS, Record);
   }

llvm/test/Bitcode/blockaddress-aggregate-users.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-bcanalyzer -dump %t.bc | FileCheck %s
+; RUN: llvm-dis %t.bc
+
+; There's a curious case where blockaddress constants may refer to functions
+; outside of the function they're used in. There's a special bitcode function
+; code, FUNC_CODE_BLOCKADDR_USERS, used to signify that this is the case.
+
+; The intent of this test is two-fold:
+; 1. Ensure we produce BLOCKADDR_USERS bitcode function code on the first fn,
+;    @repro, since @fun and @fun2 both refer to @repro via blockaddress
+;    constants.
+; 2. Ensure we can round-trip serializing+desearlizing such case.
+
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <BLOCKADDR_USERS op0=2 op1=1
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <FUNCTION_BLOCK
+
+%struct.ptrs = type { i8*, i8* }
+
+define void @repro() {
+  br label %label
+
+label:
+  call void @fun()
+  ret void
+}
+
+define void @fun() noinline {
+  call void @f(%struct.ptrs { i8* blockaddress(@repro, %label), i8* blockaddress(@repro, %label) })
+  ret void
+}
+
+define void @fun2() noinline {
+  call void @f(%struct.ptrs { i8* blockaddress(@repro, %label), i8* blockaddress(@repro, %label) })
+  ret void
+}
+
+declare void @f(%struct.ptrs)

llvm/test/Bitcode/blockaddress-expr-users.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-bcanalyzer -dump %t.bc | FileCheck %s
+; RUN: llvm-dis %t.bc
+
+; There's a curious case where blockaddress constants may refer to functions
+; outside of the function they're used in. There's a special bitcode function
+; code, FUNC_CODE_BLOCKADDR_USERS, used to signify that this is the case.
+
+; The intent of this test is two-fold:
+; 1. Ensure we produce BLOCKADDR_USERS bitcode function code on the first fn,
+;    @repro, since @fun and @fun2 both refer to @repro via blockaddress
+;    constants.
+; 2. Ensure we can round-trip serializing+desearlizing such case.
+
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <BLOCKADDR_USERS op0=2 op1=1
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <FUNCTION_BLOCK
+
+define void @repro() {
+  br label %label
+
+label:
+  call void @fun()
+  ret void
+}
+
+define void @fun() noinline {
+  call void @f(i64 ptrtoint (i8* blockaddress(@repro, %label) to i64))
+  ret void
+}
+
+define void @fun2() noinline {
+  call void @f(i64 ptrtoint (i8* blockaddress(@repro, %label) to i64))
+  ret void
+}
+
+declare void @f(i64)

llvm/test/Bitcode/blockaddress-globalvalue-users.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-bcanalyzer -dump %t.bc | FileCheck %s
+; RUN: llvm-dis %t.bc
+
+; There's a curious case where blockaddress constants may refer to functions
+; outside of the function they're used in. There's a special bitcode function
+; code, FUNC_CODE_BLOCKADDR_USERS, used to signify that this is the case.
+
+; The intent of this test is two-fold:
+; 1. Ensure we do not produce BLOCKADDR_USERS bitcode function code on the first
+;    fn, @repro, by accident, when @fun and @fun2 use a global value, @foo,
+;    which is initialized to @repro's blockaddress constants.
+; 2. Ensure we can round-trip serializing+desearlizing such case.
+
+; CHECK: <FUNCTION_BLOCK
+; CHECK-NOT: <BLOCKADDR_USERS
+
+@foo = global i8* blockaddress(@repro, %label)
+
+define void @repro() {
+  br label %label
+
+label:
+  call void @fun()
+  ret void
+}
+
+define void @fun() noinline {
+  call void @f(i8** @foo)
+  ret void
+}
+
+define void @fun2() noinline {
+  call void @f(i8** @foo)
+  ret void
+}
+
+declare void @f(i8**)
 No newline at end of file

llvm/test/Bitcode/blockaddress-nested-users.ll

+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-bcanalyzer -dump %t.bc | FileCheck %s
+; RUN: llvm-dis %t.bc
+
+; There's a curious case where blockaddress constants may refer to functions
+; outside of the function they're used in. There's a special bitcode function
+; code, FUNC_CODE_BLOCKADDR_USERS, used to signify that this is the case.
+
+; The intent of this test is two-fold:
+; 1. Ensure we produce BLOCKADDR_USERS bitcode function code on the first fn,
+;    @repro, since @fun and @fun2 both refer to @repro via blockaddress
+;    constants.
+; 2. Ensure we can round-trip serializing+desearlizing such case.
+
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <BLOCKADDR_USERS op0=1 op1=2
+; CHECK: <FUNCTION_BLOCK
+; CHECK: <FUNCTION_BLOCK
+
+define void @repro() {
+  br label %label
+
+label:
+  call void @fun()
+  ret void
+}
+
+define void @fun() noinline {
+  call void @f(i64 add (i64 ptrtoint (i8* blockaddress(@repro, %label) to i64), i64 1))
+  ret void
+}
+
+define void @fun2() noinline {
+  call void @f(i64 add (i64 ptrtoint (i8* blockaddress(@repro, %label) to i64), i64 2))
+  ret void
+}
+
+declare void @f(i64)