From a25e636ce4ecc4cb3cdef8ef4cc85781144cb655 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 13 Aug 2019 17:52:21 +0000 Subject: [PATCH] [AutoUpgrader] Make ArcRuntime Autoupgrader more conservative Summary: This is a tweak to r368311 and r368646 which auto upgrades the calls to objc runtime functions to objc runtime intrinsics, in order to make sure that the auto upgrader does not trigger with up-to-date bitcode. It is possible for bitcode that is up-to-date to contain direct calls to objc runtime function and those are not inserted by compiler as part of ARC and they should not be upgraded. Now auto upgrader only triggers as when the old style of ARC marker is used so it is guaranteed that it won't trigger on update-to-date bitcode. This also means it won't do this upgrade for bitcode from llvm-8 and llvm-9, which preserves the behavior of those releases. Ideally they should be upgraded as well but it is more important to make sure AutoUpgrader will not trigger on up-to-date bitcode. Reviewers: ahatanak, rjmccall, dexonsmith, pete Reviewed By: dexonsmith Subscribers: hiraditya, jkorous, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D66153 llvm-svn: 368730 --- include/llvm/IR/AutoUpgrade.h | 10 +-- lib/Bitcode/Reader/BitcodeReader.cpp | 3 +- lib/IR/AutoUpgrade.cpp | 13 ++-- test/Bitcode/upgrade-arc-runtime-calls-new.bc | Bin 0 -> 4720 bytes test/Bitcode/upgrade-arc-runtime-calls.ll | 70 +++++++++--------- 5 files changed, 48 insertions(+), 48 deletions(-) create mode 100644 test/Bitcode/upgrade-arc-runtime-calls-new.bc diff --git a/include/llvm/IR/AutoUpgrade.h b/include/llvm/IR/AutoUpgrade.h index 8613a1545ea..99ab68977a4 100644 --- a/include/llvm/IR/AutoUpgrade.h +++ b/include/llvm/IR/AutoUpgrade.h @@ -54,13 +54,9 @@ namespace llvm { /// module is modified. bool UpgradeModuleFlags(Module &M); - /// This checks for objc retain release marker which should be upgraded. It - /// returns true if module is modified. - bool UpgradeRetainReleaseMarker(Module &M); - - /// Convert calls to ARC runtime functions to intrinsic calls if the bitcode - /// has the arm64 retainAutoreleasedReturnValue marker. - void UpgradeARCRuntimeCalls(Module &M); + /// Convert calls to ARC runtime functions to intrinsic calls and upgrade the + /// old retain release marker to new module flag format. + void UpgradeARCRuntime(Module &M); void UpgradeSectionAttributes(Module &M); diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index cba9385318c..997b556f0f9 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -5312,8 +5312,7 @@ Error BitcodeReader::materializeModule() { UpgradeModuleFlags(*TheModule); - UpgradeRetainReleaseMarker(*TheModule); - UpgradeARCRuntimeCalls(*TheModule); + UpgradeARCRuntime(*TheModule); return Error::success(); } diff --git a/lib/IR/AutoUpgrade.cpp b/lib/IR/AutoUpgrade.cpp index e8443aeb715..e9d49d8a2b5 100644 --- a/lib/IR/AutoUpgrade.cpp +++ b/lib/IR/AutoUpgrade.cpp @@ -3830,7 +3830,9 @@ bool llvm::UpgradeDebugInfo(Module &M) { return Modified; } -bool llvm::UpgradeRetainReleaseMarker(Module &M) { +/// This checks for objc retain release marker which should be upgraded. It +/// returns true if module is modified. +static bool UpgradeRetainReleaseMarker(Module &M) { bool Changed = false; const char *MarkerKey = "clang.arc.retainAutoreleasedReturnValueMarker"; NamedMDNode *ModRetainReleaseMarker = M.getNamedMetadata(MarkerKey); @@ -3854,7 +3856,7 @@ bool llvm::UpgradeRetainReleaseMarker(Module &M) { return Changed; } -void llvm::UpgradeARCRuntimeCalls(Module &M) { +void llvm::UpgradeARCRuntime(Module &M) { // This lambda converts normal function calls to ARC runtime functions to // intrinsic calls. auto UpgradeToIntrinsic = [&](const char *OldFunc, @@ -3905,9 +3907,10 @@ void llvm::UpgradeARCRuntimeCalls(Module &M) { // "llvm.objc.clang.arc.use". UpgradeToIntrinsic("clang.arc.use", llvm::Intrinsic::objc_clang_arc_use); - // Return if the bitcode doesn't have the arm64 retainAutoreleasedReturnValue - // marker. We don't know for sure that it was compiled with ARC in that case. - if (!M.getModuleFlag("clang.arc.retainAutoreleasedReturnValueMarker")) + // Upgrade the retain release marker. If there is no need to upgrade + // the marker, that means either the module is already new enough to contain + // new intrinsics or it is not ARC. There is no need to upgrade runtime call. + if (!UpgradeRetainReleaseMarker(M)) return; std::pair RuntimeFuncs[] = { diff --git a/test/Bitcode/upgrade-arc-runtime-calls-new.bc b/test/Bitcode/upgrade-arc-runtime-calls-new.bc new file mode 100644 index 0000000000000000000000000000000000000000..68e350ab443944d649e6847f77c999cf588a0815 GIT binary patch literal 4720 zcmai$4R9036@X8c(8(Z9!q`O$l6pdgc>>y*Ckt{<@F+qS0roh-ITf!!?Bunxi zNjj1xA`xUN7&GHcNNWh8B`qD(QtD|)$F>Y{sQ7ImP^iZq1BJveo{+YgHZ$$pJDoWW zC=08-_wC!aZ{NM=z2UxD(=ce+(95$h?7z^8t$o9J0Hy%5OAT|^Z2qHj&3{g=U0tCo z5g4anS}BGVWnkE}3@ru&_8sVWNVm^2Bw8hw8}s)IBGQ+qNR%4U9(z!+BBOZ!wBkXz zG20|jte$GL$4b3}T3tz2G%mO7&!`=eoX94vD`Vtm0;4^mn6ICoSo~(s)_vsJA+Wof z#4t6C#yPP}vUZnfrtU}te#Rn)64_a0@%XgJV41wDU(%-%6bmx_qH41!=L{C1^wMST zkdn@9fxPbfA@dq#km~-qq|dxgIrW3?W0C=Lg)(UA{-tz)v~3g>cOMsvn)0xF@989Z zm82`f&~An;v2_Xz6T&zZq?S9B7TU z^$M2;;eU8KWg{9c5j!V{h6K@U#&=fZflK21a@-fhchbKNBZQw!^WW0sC2biY8$S5j zYahR5osTU;ZqO|#!^rB=DV9t*OJEd9kM)iGD99n8(}Y(FMI0osezfU&7RezJ#rk8` zNAxk$)ZCZ&LenjgaFjCME}wU~%*-Kj!;!F0VBAS z_fjf@=hEkue3$OOeSqiE)a3)DfVW*dX5pL45AjWPObrOoOG-C5-Hwe1rM%OP`o#QT z+MGWOvWs8@&wsb1g16oL-V^-o-o3#0{G*niPUmemKISrWY(p@w{A_V+kb_{mt5#}) z7ld;^85-dQ;ecs(KkpUK-+y=+$*~2mc(nY{6F~?LibIDj5YEj=Q4HOv=~5_XlqKSs z-+T3}$eQ&k7Sp4!oq!zQ4dW6?P&GKJa%KyPVpdZm87~gvZLPS!t5JFvlRc0VUk8aL z(qhd(zxu)hPs? zP-ghjU=(W4Ta7pC@#b%)fZ3K{v`P|fR78DU(YCJ8f`=2t!x7>ieZ;syXdgPHx_wBM zqJM8us&iV^U`TzA5?@PL^M|bYUG#5n($r}+#e@Z&$sfR`-?kV0MOe_2lzv(t^;SlG z-f*A}6?s4r4RlA97dFTqVzRGn#O(wzW|obT#5kOS)T?jxYX&TuD_Yg9kmlBi>h`Gm z3xUR|SZC_7E;|@5NZID54^ka%X~kQ+)|Gvokf8`@c4*E~s(V`XaKHM;0u6FltDd05 z<-5YBRI;GQS_A?4cA)UKwIBv!GJa+%oHW`3{z&khdfe9)+BzmD#`}mnHsX;%*MG&L zxjj1T`lxys94J*5^n{B}hG$2Trnogfl`QP-&mT&{84FXTCmSU=c-062tc)seo+ZYs zWe-Ng6Qt~pL3W3f{nJLc<;0kQ0P)d?`2GlS+eUl?DT1|;7`6~bpQeuU=a8;kcQv>)i@Ut$g2I9B7L+}jsXzM4foM}Vq#TbcqMpTp&#{Epi7 zjzE*oy^(S?Io43V*B!nFPjjZzyMT0hGcyaw2AYvhkB{`&chpopWq`9D(@9DcDT6cw zzGR+@b)5h8`41m-{A%y<)S39gahJQZ8hkzmd5$i>X;XcA$RB)XrMJsk1ZmJ?&A*gv zzpFZXX!<=1O7FW`HS=Du%kakz)efF2JQ6NA2+1NW>JAq|;v|x&b|@0VyfDrbC1aKu z5mGVNX^SheEt2+ryZCHOfKaZ(V7H0167edfN+(_^Z?B%{?57wlONsc?SO{sA)tIw& z;uSD!>wue)UT{@0H@zSp+MhN%NBpPsPE=3YMwU3mqDzzL0*fZ%3I*~fM=Z;=S0g`^ zio#3G?3?Ae%ubn#5@z#6Ff@tESR}|}waOXP6AGo#Y{k~Ji@|*t1&{ z>=1?aiP{f{!taVA?@MwT^u-lh-m%>{I;Z^i$0a%L1;q!p?6KXyJE#2PPb4`WL-Ez` znk~5-BAZjTt0zB`R&2Wo52r3mE8g02-gdS3hP2}SoAB_5QE9~|TQ1nH7Q*&)Yb2Sw z-$=;kM6iqKGk|Um`1ZL0Pg%mwUX%yudioH4j4+l#M>$Xl9W^wPXiTsE9Q4q4)|W8i zFtYb1I}eD~N8?wZGXp(9Lwe`~1e*&!fi1)lW4Q!)HgHI1di|`lKH>|2D?wfg9nB*< zh_3*y0(mX;*&I#*CqUi-eL9DGfTMj|pmX;PruCyd{k>`4$>RrsqwmE-&~u=({`@Ab ze~PEy2ORm4gbtyf)ISd#$rquc8fNX>0FKW8SLkRTmQMhm1M&oPEr(}`FiZ#X3;{FG z>gNGR=UoDvd)}34ZsqY!z-6Gn0XT{uYsU#(4>ARO9*6G*j&dOed?tq<29ErG4>&hI zy};4AdVq7!brCpvuV;ayd3N8w14n-U4LI_P<^KYX^4IY>=lYNcExjCWGRLGw`o5zlm`jZF$hi=oP5Evz7C6he z_bA6ivCC7#oW+l>+zoIX_jY=omM6Vya{DQ#!|I~y8(wVGQM=Z@!%-8U?KHU=U!DVP zo_cq{;p4cpi2w5ZZSI;yj!7*>Y^`U^!$(6E<#u}lRG{ADt_pa(wVoDtmEG#k=WFKQUaTF z?lY84env`XKPwWPX#f66D@ F{{Yn*q%i;h literal 0 HcmV?d00001 diff --git a/test/Bitcode/upgrade-arc-runtime-calls.ll b/test/Bitcode/upgrade-arc-runtime-calls.ll index dbccff2a5b0..d42c776ddc5 100644 --- a/test/Bitcode/upgrade-arc-runtime-calls.ll +++ b/test/Bitcode/upgrade-arc-runtime-calls.ll @@ -3,10 +3,12 @@ ; upgrade-arc-runtime-calls.bc and upgrade-mrr-runtime-calls.bc are identical ; except that the former has the arm64 retainAutoreleasedReturnValueMarker -; metadata. +; metadata. upgrade-arc-runtime-calls-new.bc has the new module flag format of +; marker, it should not be upgraded. ; RUN: llvm-dis < %S/upgrade-arc-runtime-calls.bc | FileCheck -check-prefixes=ARC %s -; RUN: llvm-dis < %S/upgrade-mrr-runtime-calls.bc | FileCheck -check-prefixes=MRR %s +; RUN: llvm-dis < %S/upgrade-mrr-runtime-calls.bc | FileCheck -check-prefixes=NOUPGRADE %s +; RUN: llvm-dis < %S/upgrade-arc-runtime-calls-new.bc | FileCheck -check-prefixes=NOUPGRADE %s define void @testRuntimeCalls(i8* %a, i8** %b, i8** %c, i32* %d, i32** %e) personality i32 (...)* @__gxx_personality_v0 { entry: @@ -89,35 +91,35 @@ unwindBlock: // ARC-NEXT: tail call void @llvm.objc.arc.annotation.bottomup.bbend(i8** %[[B]], i8** %[[C]]) // ARC-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]]) -// MRR: define void @testRuntimeCalls(i8* %[[A:.*]], i8** %[[B:.*]], i8** %[[C:.*]], i32* %[[D:.*]], i32** %[[E:.*]]) personality -// MRR: %[[V0:.*]] = tail call i8* @objc_autorelease(i8* %[[A]]) -// MRR-NEXT: tail call void @objc_autoreleasePoolPop(i8* %[[A]]) -// MRR-NEXT: %[[V1:.*]] = tail call i8* @objc_autoreleasePoolPush() -// MRR-NEXT: %[[V2:.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* %[[A]]) -// MRR-NEXT: tail call void @objc_copyWeak(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: tail call void @objc_destroyWeak(i8** %[[B]]) -// MRR-NEXT: %[[V3:.*]] = tail call i32* @objc_initWeak(i32** %[[E]], i32* %[[D]]) -// MRR-NEXT: %[[V4:.*]] = tail call i8* @objc_loadWeak(i8** %[[B]]) -// MRR-NEXT: %[[V5:.*]] = tail call i8* @objc_loadWeakRetained(i8** %[[B]]) -// MRR-NEXT: tail call void @objc_moveWeak(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: tail call void @objc_release(i8* %[[A]]) -// MRR-NEXT: %[[V6:.*]] = tail call i8* @objc_retain(i8* %[[A]]) -// MRR-NEXT: %[[V7:.*]] = tail call i8* @objc_retainAutorelease(i8* %[[A]]) -// MRR-NEXT: %[[V8:.*]] = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %[[A]]) -// MRR-NEXT: %[[V9:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %[[A]]) -// MRR-NEXT: %[[V10:.*]] = tail call i8* @objc_retainBlock(i8* %[[A]]) -// MRR-NEXT: tail call void @objc_storeStrong(i8** %[[B]], i8* %[[A]]) -// MRR-NEXT: %[[V11:.*]] = tail call i8* @objc_storeWeak(i8** %[[B]], i8* %[[A]]) -// MRR-NEXT: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[A]]) -// MRR-NEXT: %[[V12:.*]] = tail call i8* @objc_unsafeClaimAutoreleasedReturnValue(i8* %[[A]]) -// MRR-NEXT: %[[V13:.*]] = tail call i8* @objc_retainedObject(i8* %[[A]]) -// MRR-NEXT: %[[V14:.*]] = tail call i8* @objc_unretainedObject(i8* %[[A]]) -// MRR-NEXT: %[[V15:.*]] = tail call i8* @objc_unretainedPointer(i8* %[[A]]) -// MRR-NEXT: %[[V16:.*]] = tail call i8* @objc_retain.autorelease(i8* %[[A]]) -// MRR-NEXT: %[[V17:.*]] = tail call i32 @objc_sync.enter(i8* %[[A]]) -// MRR-NEXT: %[[V18:.*]] = tail call i32 @objc_sync.exit(i8* %[[A]]) -// MRR-NEXT: tail call void @objc_arc_annotation_topdown_bbstart(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: tail call void @objc_arc_annotation_topdown_bbend(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: tail call void @objc_arc_annotation_bottomup_bbstart(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: tail call void @objc_arc_annotation_bottomup_bbend(i8** %[[B]], i8** %[[C]]) -// MRR-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]]) +// NOUPGRADE: define void @testRuntimeCalls(i8* %[[A:.*]], i8** %[[B:.*]], i8** %[[C:.*]], i32* %[[D:.*]], i32** %[[E:.*]]) personality +// NOUPGRADE: %[[V0:.*]] = tail call i8* @objc_autorelease(i8* %[[A]]) +// NOUPGRADE-NEXT: tail call void @objc_autoreleasePoolPop(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V1:.*]] = tail call i8* @objc_autoreleasePoolPush() +// NOUPGRADE-NEXT: %[[V2:.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* %[[A]]) +// NOUPGRADE-NEXT: tail call void @objc_copyWeak(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: tail call void @objc_destroyWeak(i8** %[[B]]) +// NOUPGRADE-NEXT: %[[V3:.*]] = tail call i32* @objc_initWeak(i32** %[[E]], i32* %[[D]]) +// NOUPGRADE-NEXT: %[[V4:.*]] = tail call i8* @objc_loadWeak(i8** %[[B]]) +// NOUPGRADE-NEXT: %[[V5:.*]] = tail call i8* @objc_loadWeakRetained(i8** %[[B]]) +// NOUPGRADE-NEXT: tail call void @objc_moveWeak(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: tail call void @objc_release(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V6:.*]] = tail call i8* @objc_retain(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V7:.*]] = tail call i8* @objc_retainAutorelease(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V8:.*]] = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V9:.*]] = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V10:.*]] = tail call i8* @objc_retainBlock(i8* %[[A]]) +// NOUPGRADE-NEXT: tail call void @objc_storeStrong(i8** %[[B]], i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V11:.*]] = tail call i8* @objc_storeWeak(i8** %[[B]], i8* %[[A]]) +// NOUPGRADE-NEXT: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V12:.*]] = tail call i8* @objc_unsafeClaimAutoreleasedReturnValue(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V13:.*]] = tail call i8* @objc_retainedObject(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V14:.*]] = tail call i8* @objc_unretainedObject(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V15:.*]] = tail call i8* @objc_unretainedPointer(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V16:.*]] = tail call i8* @objc_retain.autorelease(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V17:.*]] = tail call i32 @objc_sync.enter(i8* %[[A]]) +// NOUPGRADE-NEXT: %[[V18:.*]] = tail call i32 @objc_sync.exit(i8* %[[A]]) +// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_topdown_bbstart(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_topdown_bbend(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_bottomup_bbstart(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: tail call void @objc_arc_annotation_bottomup_bbend(i8** %[[B]], i8** %[[C]]) +// NOUPGRADE-NEXT: invoke void @objc_autoreleasePoolPop(i8* %[[A]])