Bug 1483123 - Apply miscompilation fix from clang upstream. r=froydnj
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 14 Aug 2018 15:35:28 +0900
changeset 486781 0d04a3f89940536d56cd5415a78316f0960e9f0a
parent 486780 e6a44943b17774c94e3c22e85e5260b0a62121ac
child 486782 4609d0b65d7c76a546c535dd1a604c4ba396f876
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1483123
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1483123 - Apply miscompilation fix from clang upstream. r=froydnj
build/build-clang/clang-6-linux64.json
build/build-clang/clang-6-macosx64.json
build/build-clang/clang-7-pre-linux64.json
build/build-clang/clang-7-pre-mingw.json
build/build-clang/clang-win64.json
build/build-clang/r339636.patch
--- a/build/build-clang/clang-6-linux64.json
+++ b/build/build-clang/clang-6-linux64.json
@@ -13,11 +13,12 @@
     "python_path": "/usr/bin/python2.7",
     "gcc_dir": "/builds/worker/workspace/build/src/gcc",
     "cc": "/builds/worker/workspace/build/src/gcc/bin/gcc",
     "cxx": "/builds/worker/workspace/build/src/gcc/bin/g++",
     "as": "/builds/worker/workspace/build/src/gcc/bin/gcc",
     "patches": [
       "find_symbolizer_linux.patch",
       "r322401.patch",
-      "r325356.patch"
+      "r325356.patch",
+      "r339636.patch"
     ]
 }
--- a/build/build-clang/clang-6-macosx64.json
+++ b/build/build-clang/clang-6-macosx64.json
@@ -19,11 +19,12 @@
     "ar": "/builds/worker/workspace/build/src/cctools/bin/x86_64-apple-darwin11-ar",
     "ranlib": "/builds/worker/workspace/build/src/cctools/bin/x86_64-apple-darwin11-ranlib",
     "libtool": "/builds/worker/workspace/build/src/cctools/bin/x86_64-apple-darwin11-libtool",
     "ld": "/builds/worker/workspace/build/src/clang/bin/clang",
     "patches": [
       "compiler-rt-cross-compile.patch",
       "compiler-rt-no-codesign.patch",
       "r322401.patch",
-      "r325356.patch"
+      "r325356.patch",
+      "r339636.patch"
     ]
 }
--- a/build/build-clang/clang-7-pre-linux64.json
+++ b/build/build-clang/clang-7-pre-linux64.json
@@ -12,11 +12,12 @@
     "libcxxabi_repo": "https://llvm.org/svn/llvm-project/libcxxabi/tags/RELEASE_700/rc1",
     "python_path": "/usr/bin/python2.7",
     "gcc_dir": "/builds/worker/workspace/build/src/gcc",
     "cc": "/builds/worker/workspace/build/src/gcc/bin/gcc",
     "cxx": "/builds/worker/workspace/build/src/gcc/bin/g++",
     "as": "/builds/worker/workspace/build/src/gcc/bin/gcc",
     "patches": [
       "find_symbolizer_linux.patch",
-      "rename_gcov_flush.patch"
+      "rename_gcov_flush.patch",
+      "r339636.patch"
     ]
 }
--- a/build/build-clang/clang-7-pre-mingw.json
+++ b/build/build-clang/clang-7-pre-mingw.json
@@ -9,10 +9,13 @@
     "lld_repo": "https://llvm.org/svn/llvm-project/lld/tags/RELEASE_700/rc1",
     "compiler_repo": "https://llvm.org/svn/llvm-project/compiler-rt/tags/RELEASE_700/rc1",
     "libcxx_repo": "https://llvm.org/svn/llvm-project/libcxx/tags/RELEASE_700/rc1",
     "libcxxabi_repo": "https://llvm.org/svn/llvm-project/libcxxabi/tags/RELEASE_700/rc1",
     "python_path": "/usr/bin/python2.7",
     "gcc_dir": "/builds/worker/workspace/build/src/gcc",
     "cc": "/builds/worker/workspace/build/src/gcc/bin/gcc",
     "cxx": "/builds/worker/workspace/build/src/gcc/bin/g++",
-    "as": "/builds/worker/workspace/build/src/gcc/bin/gcc"
+    "as": "/builds/worker/workspace/build/src/gcc/bin/gcc",
+    "patches": [
+      "r339636.patch"
+    ]
 }
--- a/build/build-clang/clang-win64.json
+++ b/build/build-clang/clang-win64.json
@@ -9,11 +9,12 @@
     "lld_repo": "https://llvm.org/svn/llvm-project/lld/trunk",
     "compiler_repo": "https://llvm.org/svn/llvm-project/compiler-rt/trunk",
     "libcxx_repo": "https://llvm.org/svn/llvm-project/libcxx/trunk",
     "python_path": "c:/mozilla-build/python/python.exe",
     "cc": "cl.exe",
     "cxx": "cl.exe",
     "ml": "ml64.exe",
     "patches": [
-      "loosen-msvc-detection.patch"
+      "loosen-msvc-detection.patch",
+      "r339636.patch"
     ]
 }
new file mode 100644
--- /dev/null
+++ b/build/build-clang/r339636.patch
@@ -0,0 +1,110 @@
+From 3ea7b0a0b1e41cf3871891919a3a1567f3865d42 Mon Sep 17 00:00:00 2001
+From: Reid Kleckner <rnk@google.com>
+Date: Tue, 14 Aug 2018 01:24:35 +0000
+Subject: [PATCH] [BasicAA] Don't assume tail calls with byval don't alias
+ allocas
+
+Summary:
+Calls marked 'tail' cannot read or write allocas from the current frame
+because the current frame might be destroyed by the time they run.
+However, a tail call may use an alloca with byval. Calling with byval
+copies the contents of the alloca into argument registers or stack
+slots, so there is no lifetime issue. Tail calls never modify allocas,
+so we can return just ModRefInfo::Ref.
+
+Fixes PR38466, a longstanding bug.
+
+Reviewers: hfinkel, nlewycky, gbiv, george.burgess.iv
+
+Subscribers: hiraditya, llvm-commits
+
+Differential Revision: https://reviews.llvm.org/D50679
+
+git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@339636 91177308-0d34-0410-b5e6-96231b3b80d8
+---
+ lib/Analysis/BasicAliasAnalysis.cpp           | 13 ++++++-----
+ test/Analysis/BasicAA/tail-byval.ll           | 15 ++++++++++++
+ .../DeadStoreElimination/tail-byval.ll        | 23 +++++++++++++++++++
+ 3 files changed, 45 insertions(+), 6 deletions(-)
+ create mode 100644 test/Analysis/BasicAA/tail-byval.ll
+ create mode 100644 test/Transforms/DeadStoreElimination/tail-byval.ll
+
+diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+index 1a24ae3dba1..f9ecbc04326 100644
+--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
++++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+@@ -801,14 +801,15 @@ ModRefInfo BasicAAResult::getModRefInfo(ImmutableCallSite CS,
+ 
+   const Value *Object = GetUnderlyingObject(Loc.Ptr, DL);
+ 
+-  // If this is a tail call and Loc.Ptr points to a stack location, we know that
+-  // the tail call cannot access or modify the local stack.
+-  // We cannot exclude byval arguments here; these belong to the caller of
+-  // the current function not to the current function, and a tail callee
+-  // may reference them.
++  // Calls marked 'tail' cannot read or write allocas from the current frame
++  // because the current frame might be destroyed by the time they run. However,
++  // a tail call may use an alloca with byval. Calling with byval copies the
++  // contents of the alloca into argument registers or stack slots, so there is
++  // no lifetime issue.
+   if (isa<AllocaInst>(Object))
+     if (const CallInst *CI = dyn_cast<CallInst>(CS.getInstruction()))
+-      if (CI->isTailCall())
++      if (CI->isTailCall() &&
++          !CI->getAttributes().hasAttrSomewhere(Attribute::ByVal))
+         return ModRefInfo::NoModRef;
+ 
+   // If the pointer is to a locally allocated object that does not escape,
+diff --git a/llvm/test/Analysis/BasicAA/tail-byval.ll b/llvm/test/Analysis/BasicAA/tail-byval.ll
+new file mode 100644
+index 00000000000..0aa8dfdaedf
+--- /dev/null
++++ b/llvm/test/Analysis/BasicAA/tail-byval.ll
+@@ -0,0 +1,15 @@
++; RUN: opt -basicaa -aa-eval -print-all-alias-modref-info -disable-output < %s 2>&1 | FileCheck %s
++
++declare void @takebyval(i32* byval %p)
++
++define i32 @tailbyval() {
++entry:
++  %p = alloca i32
++  store i32 42, i32* %p
++  tail call void @takebyval(i32* byval %p)
++  %rv = load i32, i32* %p
++  ret i32 %rv
++}
++; FIXME: This should be Just Ref.
++; CHECK-LABEL: Function: tailbyval: 1 pointers, 1 call sites
++; CHECK-NEXT:   Both ModRef:  Ptr: i32* %p       <->  tail call void @takebyval(i32* byval %p)
+diff --git a/llvm/test/Transforms/DeadStoreElimination/tail-byval.ll b/llvm/test/Transforms/DeadStoreElimination/tail-byval.ll
+new file mode 100644
+index 00000000000..ed2fbd434a7
+--- /dev/null
++++ b/llvm/test/Transforms/DeadStoreElimination/tail-byval.ll
+@@ -0,0 +1,23 @@
++; RUN: opt -dse -S < %s | FileCheck %s
++
++; Don't eliminate stores to allocas before tail calls to functions that use
++; byval. It's correct to mark calls like these as 'tail'. To implement this tail
++; call, the backend should copy the bytes from the alloca into the argument area
++; before clearing the stack.
++
++target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
++target triple = "i386-unknown-linux-gnu"
++
++declare void @g(i32* byval %p)
++
++define void @f(i32* byval %x) {
++entry:
++  %p = alloca i32
++  %v = load i32, i32* %x
++  store i32 %v, i32* %p
++  tail call void @g(i32* byval %p)
++  ret void
++}
++; CHECK-LABEL: define void @f(i32* byval %x)
++; CHECK:   store i32 %v, i32* %p
++; CHECK:   tail call void @g(i32* byval %p)
+-- 
+2.18.0
+