From owner-svn-src-stable-11@freebsd.org Wed Aug 22 17:36:28 2018 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2BEFF108FF9D; Wed, 22 Aug 2018 17:36:28 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D56DD805D3; Wed, 22 Aug 2018 17:36:27 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B7503162DA; Wed, 22 Aug 2018 17:36:27 +0000 (UTC) (envelope-from dim@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w7MHaRpd001742; Wed, 22 Aug 2018 17:36:27 GMT (envelope-from dim@FreeBSD.org) Received: (from dim@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w7MHaRiJ001741; Wed, 22 Aug 2018 17:36:27 GMT (envelope-from dim@FreeBSD.org) Message-Id: <201808221736.w7MHaRiJ001741@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: dim set sender to dim@FreeBSD.org using -f From: Dimitry Andric Date: Wed, 22 Aug 2018 17:36:27 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r338207 - stable/11/contrib/llvm/lib/Target/X86 X-SVN-Group: stable-11 X-SVN-Commit-Author: dim X-SVN-Commit-Paths: stable/11/contrib/llvm/lib/Target/X86 X-SVN-Commit-Revision: 338207 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Aug 2018 17:36:28 -0000 Author: dim Date: Wed Aug 22 17:36:27 2018 New Revision: 338207 URL: https://svnweb.freebsd.org/changeset/base/338207 Log: MFC r337615: Pull in r338481 from upstream llvm trunk (by Chandler Carruth): [x86] Fix a really subtle miscompile due to a somewhat glaring bug in EFLAGS copy lowering. If you have a branch of LLVM, you may want to cherrypick this. It is extremely unlikely to hit this case empirically, but it will likely manifest as an "impossible" branch being taken somewhere, and will be ... very hard to debug. Hitting this requires complex conditions living across complex control flow combined with some interesting memory (non-stack) initialized with the results of a comparison. Also, because you have to arrange for an EFLAGS copy to be in *just* the right place, almost anything you do to the code will hide the bug. I was unable to reduce anything remotely resembling a "good" test case from the place where I hit it, and so instead I have constructed synthetic MIR testing that directly exercises the bug in question (as well as the good behavior for completeness). The issue is that we would mistakenly assume any SETcc with a valid condition and an initial operand that was a register and a virtual register at that to be a register *defining* SETcc... It isn't though.... This would in turn cause us to test some other bizarre register, typically the base pointer of some memory. Now, testing this register and using that to branch on doesn't make any sense. It even fails the machine verifier (if you are running it) due to the wrong register class. But it will make it through LLVM, assemble, and it *looks* fine... But wow do you get a very unsual and surprising branch taken in your actual code. The fix is to actually check what kind of SETcc instruction we're dealing with. Because there are a bunch of them, I just test the may-store bit in the instruction. I've also added an assert for sanity that ensure we are, in fact, *defining* the register operand. =D Noticed by: kib Modified: stable/11/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp Directory Properties: stable/11/ (props changed) Modified: stable/11/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp ============================================================================== --- stable/11/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp Wed Aug 22 16:32:53 2018 (r338206) +++ stable/11/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp Wed Aug 22 17:36:27 2018 (r338207) @@ -608,9 +608,12 @@ X86FlagsCopyLoweringPass::collectCondsInRegs(MachineBa for (MachineInstr &MI : llvm::reverse( llvm::make_range(MBB.instr_begin(), CopyDefI.getIterator()))) { X86::CondCode Cond = X86::getCondFromSETOpc(MI.getOpcode()); - if (Cond != X86::COND_INVALID && MI.getOperand(0).isReg() && - TRI->isVirtualRegister(MI.getOperand(0).getReg())) + if (Cond != X86::COND_INVALID && !MI.mayStore() && MI.getOperand(0).isReg() && + TRI->isVirtualRegister(MI.getOperand(0).getReg())) { + assert(MI.getOperand(0).isDef() && + "A non-storing SETcc should always define a register!"); CondRegs[Cond] = MI.getOperand(0).getReg(); + } // Stop scanning when we see the first definition of the EFLAGS as prior to // this we would potentially capture the wrong flag state.