Enable including llvm and clang headers in C++20 projects without compilation errors#4
Merged
pfez merged 0 commit intorevng:masterfrom Mar 6, 2021
Merged
Conversation
pfez
force-pushed
the
feature/enable-build-as-c++20
branch
from
b11fc0e to
fac79c2
Compare
tvandera
pushed a commit
to tvandera/llvm-project
that referenced
this pull request
Oct 5, 2021
Andrei Matei reported a llvm11 core dump for his bpf program https://bugs.llvm.org/show_bug.cgi?id=48578 The core dump happens in LiveVariables analysis phase. revng#4 0x00007fce54356bb0 __restore_rt revng#5 0x00007fce4d51785e llvm::LiveVariables::HandleVirtRegUse(unsigned int, llvm::MachineBasicBlock*, llvm::MachineInstr&) revng#6 0x00007fce4d519abe llvm::LiveVariables::runOnInstr(llvm::MachineInstr&, llvm::SmallVectorImpl<unsigned int>&) revng#7 0x00007fce4d519ec6 llvm::LiveVariables::runOnBlock(llvm::MachineBasicBlock*, unsigned int) revng#8 0x00007fce4d51a4bf llvm::LiveVariables::runOnMachineFunction(llvm::MachineFunction&) The bug can be reproduced with llvm12 and latest trunk as well. Futher analysis shows that there is a bug in BPF peephole TRUNC elimination optimization, which tries to remove unnecessary TRUNC operations (a <<= 32; a >>= 32). Specifically, the compiler did wrong transformation for the following patterns: %1 = LDW ... %2 = SLL_ri %1, 32 %3 = SRL_ri %2, 32 ... %3 ... %4 = SRA_ri %2, 32 ... %4 ... The current transformation did not check how many uses of %2 and did transformation like %1 = LDW ... ... %1 ... %4 = SRL_ri %2, 32 ... %4 ... and pseudo register %2 is used by not defined and caused LiveVariables analysis core dump. To fix the issue, when traversing back from SRL_ri to SLL_ri, check to ensure SLL_ri has only one use. Otherwise, don't do transformation. Differential Revision: https://reviews.llvm.org/D97792 (cherry picked from commit 51cdb78)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I just backported a few patches from upstream.
Until now we had a bunch of workarounds to stop most compiler complaints about llvm and clang headers when they were compiled as part of a C++20 project, like revng.
See 7ddd738 and 75d3090
Sadly, those commits were handrcrafted by us, and did not cover all cases. Sometimes we would include a new llvm or clang header and start getting a bunch of new errors and warnings.
For some refactoring that is happening internally in revng-c and caliban, we suddenly needed some clang headers that still refused to compile as C++20. Before rolling yet another custom patch I checked upstream and I found that they have finally implemented whet we need, and I backported it. The backported stuff makes our custom patches mentioned above entirely obsolete, so I just dropped them in this rebase.
Finally, I added a commit c8a20bc to silence a bunch of new warnings that were triggered by compiling some of the mentioned clang headers with the strict warnings we have in revng.