Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addfc38672c73efd5c4e559a2e455b086e3e20c5 causes boot issues with arm64 defconfig #649

Closed
nathanchance opened this issue Sep 1, 2019 · 6 comments

Comments

@nathanchance
Copy link
Member

@nathanchance nathanchance commented Sep 1, 2019

https://git.kernel.org/next/linux-next/c/addfc38672c73efd5c4e559a2e455b086e3e20c5

The boot just hangs: https://travis-ci.com/ClangBuiltLinux/continuous-integration/jobs/229812606

git bisect log
# bad: [6d028043b55e54f48fbdf62ea8ce11a4ad830cac] Add linux-next specific files for 20190830
# good: [4a64489cf8e21a17fd6dd88935818ba7307ba996] Merge tag 'Wimplicit-fallthrough-5.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
git bisect start 'origin/master' 'origin/stable'
# bad: [199d454c0775386a645dd9e80b486c346816762f] Merge remote-tracking branch 'crypto/master'
git bisect bad 199d454c0775386a645dd9e80b486c346816762f
# bad: [0fdcb1ec5bc8e5a40eb6ede7ac1071cfae01a3c0] Merge remote-tracking branch 'hid/for-next'                                                                                           
git bisect bad 0fdcb1ec5bc8e5a40eb6ede7ac1071cfae01a3c0
# bad: [858364b05a8dc9674b93022eab1987b8671c2dc5] Merge remote-tracking branch 'tegra/for-next'
git bisect bad 858364b05a8dc9674b93022eab1987b8671c2dc5
# bad: [69c8ca5334edaa9ec384ab1f474ff2192fde686d] Merge remote-tracking branch 'actions/for-next'
git bisect bad 69c8ca5334edaa9ec384ab1f474ff2192fde686d
# good: [10826e8d36de850875acc8bf1cb8a58b3dd5e313] Merge remote-tracking branch 'dma-mapping/for-next'
git bisect good 10826e8d36de850875acc8bf1cb8a58b3dd5e313
# good: [e7e0da219c21d58170971e07c469a632ee258d94] Merge branches 'arm/soc', 'arm/fixes' and 'arm/drivers' into for-next
git bisect good e7e0da219c21d58170971e07c469a632ee258d94                                                                                                                                
# good: [98ee6cd3ed6691a2fa8f79f58824604c7bd6d2d3] Merge branches 'for-next/52-bit-kva', 'for-next/cpu-topology', 'for-next/error-injection', 'for-next/psci-cpuidle', 'for-next/rng', 'for-next/smpboot', 'for-next/tbi' and 'for-next/tlbi' into for-next/core
git bisect good 98ee6cd3ed6691a2fa8f79f58824604c7bd6d2d3
# good: [a39e44c197821f1cd54fd191db8454f3d51548ad] Merge remote-tracking branch 'arm/for-next'
git bisect good a39e44c197821f1cd54fd191db8454f3d51548ad
# bad: [ff560c0d79d559b1918a4b678eb810e00a578c35] arm64: atomics: Use K constraint when toolchain appears to support it
git bisect bad ff560c0d79d559b1918a4b678eb810e00a578c35
# bad: [eb3aabbfbfc203082d06a64517df97a3746ba9ea] arm64: atomics: Remove atomic_ll_sc compilation unit
git bisect bad eb3aabbfbfc203082d06a64517df97a3746ba9ea
# good: [580fa1b874711d633f9b145b7777b0e83ebf3787] arm64: Use correct ll/sc atomic constraints
git bisect good 580fa1b874711d633f9b145b7777b0e83ebf3787
# bad: [3337cb5aea594e4090a660e3fc3250bb669b1305] arm64: avoid using hard-coded registers for LSE atomics
git bisect bad 3337cb5aea594e4090a660e3fc3250bb669b1305
# bad: [addfc38672c73efd5c4e559a2e455b086e3e20c5] arm64: atomics: avoid out-of-line ll/sc atomics
git bisect bad addfc38672c73efd5c4e559a2e455b086e3e20c5
# first bad commit: [addfc38672c73efd5c4e559a2e455b086e3e20c5] arm64: atomics: avoid out-of-line ll/sc atomics
@nickdesaulniers nickdesaulniers self-assigned this Sep 4, 2019
@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers commented Sep 4, 2019

Not an outright fix, but today I worked on improving LLVM's verification of the IR representation of asm goto so that we should be able to catch this kind of invalidated invariant easier in the future. It should also simply finding the bad transform: https://reviews.llvm.org/D67196

@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers commented Sep 5, 2019

llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Sep 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

llvm-svn: 371262
dtzWill pushed a commit to llvm-mirror/llvm that referenced this issue Sep 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371262 91177308-0d34-0410-b5e6-96231b3b80d8
chapuni pushed a commit to llvm-project/llvm-project-20170507 that referenced this issue Sep 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252
chapuni pushed a commit to llvm-project/llvm-project-submodule that referenced this issue Sep 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252
@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers commented Sep 6, 2019

Fixed: https://reviews.llvm.org/rL371262

Now to make sure we get it in the clang-9 release (cc @zmodem)

earl pushed a commit to earl/llvm-mirror that referenced this issue Sep 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371262 91177308-0d34-0410-b5e6-96231b3b80d8
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Sep 9, 2019
------------------------------------------------------------------------
r371262 | nickdesaulniers | 2019-09-06 23:50:11 +0200 (Fri, 06 Sep 2019) | 45 lines

[IR] CallBrInst: scan+update arg list when indirect dest list changes

Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252
------------------------------------------------------------------------

llvm-svn: 371376
@zmodem
Copy link

@zmodem zmodem commented Sep 9, 2019

I've merged the commit to release_90 and replied on https://bugs.llvm.org/show_bug.cgi?id=43222 with the details.

dtzWill pushed a commit to llvm-mirror/llvm that referenced this issue Sep 9, 2019
------------------------------------------------------------------------
r371262 | nickdesaulniers | 2019-09-06 23:50:11 +0200 (Fri, 06 Sep 2019) | 45 lines

[IR] CallBrInst: scan+update arg list when indirect dest list changes

Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252
------------------------------------------------------------------------


git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_90@371376 91177308-0d34-0410-b5e6-96231b3b80d8
JaxLinAMD pushed a commit to GPUOpen-Drivers/llvm that referenced this issue Sep 25, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371262 91177308-0d34-0410-b5e6-96231b3b80d8
earl pushed a commit to earl/llvm-mirror that referenced this issue Sep 25, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@372923 91177308-0d34-0410-b5e6-96231b3b80d8
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Sep 25, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196

llvm-svn: 372923
dtzWill pushed a commit to llvm-mirror/llvm that referenced this issue Sep 25, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@372923 91177308-0d34-0410-b5e6-96231b3b80d8
chapuni pushed a commit to llvm-project/llvm-project-20170507 that referenced this issue Sep 25, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196
chapuni pushed a commit to llvm-project/llvm-project-submodule that referenced this issue Sep 25, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196
jrtc27 added a commit to CTSRD-CHERI/llvm-project that referenced this issue Oct 5, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

llvm-svn: 371262
lialan added a commit to etclabscore/evm_llvm that referenced this issue Nov 6, 2019
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371262 91177308-0d34-0410-b5e6-96231b3b80d8
arichardson added a commit to arichardson/llvm-project that referenced this issue Nov 16, 2019
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196

llvm-svn: 372923
da-x pushed a commit to da-x/llvm-project that referenced this issue Nov 19, 2019
…st changes

Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

llvm-svn: 371262
(cherry picked from commit 7a7bba2)
Change-Id: If0be070043c54384cb7aa5fd5dd22224ede4dacf
yeewang pushed a commit to yeewang/llvm-project that referenced this issue Feb 4, 2020
…st changes

Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

llvm-svn: 371262
(cherry picked from commit 7a7bba2)

Change-Id: I3d3a3dcf92c148e4e57653ea086c2febddbc4e38
JohnHolmesII pushed a commit to JohnHolmesII/llvm-project that referenced this issue Oct 12, 2020
Summary:
There's an unspoken invariant of callbr that the list of BlockAddress
Constants in the "function args" list match the BasicBlocks in the
"other labels" list. (This invariant is being added to the LangRef in
https://reviews.llvm.org/D67196).

When modifying the any of the indirect destinations of a callbr
instruction (possible jump targets), we need to update the function
arguments if the argument is a BlockAddress whose BasicBlock refers to
the indirect destination BasicBlock being replaced.  Otherwise, many
transforms that modify successors will end up violating that invariant.
A recent change to the arm64 Linux kernel exposed this bug, which
prevents the kernel from booting.

I considered maintaining a mapping from indirect destination BasicBlock
to argument operand BlockAddress, but this ends up being a one to
potentially many (though usually one) mapping.  Also, the list of
arguments to a function (or more typically inline assembly) ends up
being less than 10.  The implementation is significantly simpler to just
rescan the full list of arguments. Because of the one to potentially
many relationship, the full arg list must be scanned (we can't stop at
the first instance).

Thanks to the following folks that reported the issue and helped debug
it:
* Nathan Chancellor
* Will Deacon
* Andrew Murray
* Craig Topper

Link: https://bugs.llvm.org/show_bug.cgi?id=43222
Link: ClangBuiltLinux/linux#649
Link: https://lists.infradead.org/pipermail/linux-arm-kernel/2019-September/678330.html

Reviewers: craig.topper, chandlerc

Reviewed By: craig.topper

Subscribers: void, javed.absar, kristof.beyls, hiraditya, llvm-commits, nathanchance, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67252

llvm-svn: 371262
JohnHolmesII pushed a commit to JohnHolmesII/llvm-project that referenced this issue Oct 12, 2020
Summary:
The list of indirect labels should ALWAYS have their blockaddresses as
argument operands to the callbr (but not necessarily the other way
around).  Add an invariant that checks this.

The verifier catches a bad test case that was added recently in r368478.
I think that was a simple mistake, and the test was made less strict in
regards to the precise addresses (as those weren't specifically the
point of the test).

This invariant will be used to find a reported bug.

Link: https://www.spinics.net/lists/arm-kernel/msg753473.html
Link: ClangBuiltLinux/linux#649

Reviewers: craig.topper, void, chandlerc

Reviewed By: void

Subscribers: ychen, lebedev.ri, javed.absar, kristof.beyls, hiraditya, llvm-commits, srhines

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67196

llvm-svn: 372923
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants