Skip to content

Commit

Permalink
[Bitcode] Include indirect users of BlockAddresses in bitcode
Browse files Browse the repository at this point in the history
The original fix (commit 23ec578) of
llvm#52787 only adds `Function`s
that have `Instruction`s that directly use `BlockAddress`es 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, `BlockAddress`es (e.g.,
`i8* blockaddress(@repro, %label)`) may first compose `ConstantExpr`s
(e.g., `i64 ptrtoint (i8* blockaddress(@repro, %label) to i64)`) and
then used by `Instruction`s. This case is not handled by the original
fix.

This patch adds *indirect* users of `BlockAddress`es, i.e., the
`Instruction`s using some `Constant`s which further use the
`BlockAddress`es, into the bitcode as well, by doing depth-first
searches.

Fixes: llvm#52787
Fixes: 23ec578 ("[Bitcode] materialize Functions early when BlockAddress taken")

Reviewed By: nickdesaulniers

Differential Revision: https://reviews.llvm.org/D124878
  • Loading branch information
twd2 authored and nickdesaulniers committed May 10, 2022
1 parent fc58d7a commit 6baaad7
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 6 deletions.
26 changes: 20 additions & 6 deletions llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#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"
Expand Down Expand Up @@ -3365,7 +3366,7 @@ void ModuleBitcodeWriter::writeFunction(
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) {
Expand Down Expand Up @@ -3401,11 +3402,24 @@ void ModuleBitcodeWriter::writeFunction(
}

if (BlockAddress *BA = BlockAddress::lookup(&BB)) {
for (User *U : BA->users()) {
if (auto *I = dyn_cast<Instruction>(U)) {
Function *P = I->getParent()->getParent();
if (P != &F)
BlockAddressUsers.insert(P);
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);
}
}
}
}
Expand Down
40 changes: 40 additions & 0 deletions llvm/test/Bitcode/blockaddress-aggregate-users.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
; 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)
38 changes: 38 additions & 0 deletions llvm/test/Bitcode/blockaddress-expr-users.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
; 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)
38 changes: 38 additions & 0 deletions llvm/test/Bitcode/blockaddress-globalvalue-users.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
; 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**)
38 changes: 38 additions & 0 deletions llvm/test/Bitcode/blockaddress-nested-users.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
; 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)

0 comments on commit 6baaad7

Please sign in to comment.