DataFlowSanitizer: wrappers of functions with local linkage should have the same linkage as the function being wrapped

[skerner]

Summary

This patch resolves link errors when the address of a static function is taken, and that function is uninstrumented by DFSan.

This change resolves bug 36314.

[skerner]

Revert a file which was mistakenly included.

[pcc]

It seems like a better fix would be to give the dfsw$ function internal linkage if the wrapped function has internal linkage. That way, if two translation units have static uninstrumented functions with the same name, the references to the dfsw$ functions will be correct.

[skerner]

Address review comment: Wrappers of local functions should have the same linkage as the function being wrapped.

[skerner]

In D44784#1045790, @pcc wrote:

It seems like a better fix would be to give the dfsw$ function internal linkage if the wrapped function has internal linkage. That way, if two translation units have static uninstrumented functions with the same name, the references to the dfsw$ functions will be correct.

Good point. Done.

[skerner]

Peter, friendly ping.

[pcc]

LGTM

I will commit.

llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

if (isInstrumented(&F)) {
       }
     } else if (!IsZeroArgsVoidRet || getWrapperKind(&F) == WK_Custom) {
       // Build a wrapper function for F.  The wrapper simply calls F, and is
       // added to FnsToInstrument so that any instrumentation according to its
       // WrapperKind is done in the second pass below.
       FunctionType *NewFT = getInstrumentedABI() == IA_Args
                                 ? getArgsFunctionType(FT)
                                 : FT;
+
+      // If the function being wrapped has local linkage, then preserve the
+      // function's linkage in the wrapper function.
+      GlobalValue::LinkageTypes wrapperLinkage =
+          F.hasLocalLinkage()
+              ? F.getLinkage()
+              : GlobalValue::LinkOnceODRLinkage;
+
       Function *NewF = buildWrapperFunction(
           &F, std::string("dfsw$") + std::string(F.getName()),
-          GlobalValue::LinkOnceODRLinkage, NewFT);
+          wrapperLinkage, NewFT);
       if (getInstrumentedABI() == IA_TLS)
         NewF->removeAttributes(AttributeList::FunctionIndex, ReadOnlyNoneAttrs);
 
       Value *WrappedFnCst =
           ConstantExpr::getBitCast(NewF, PointerType::getUnqual(FT));
       F.replaceAllUsesWith(WrappedFnCst);
 
       UnwrappedFnMap[WrappedFnCst] = &F;

llvm/trunk/test/Instrumentation/DataFlowSanitizer/Inputs/abilist.txt

 fun:discard*=uninstrumented
 fun:discard*=discard
 
 fun:functional=uninstrumented
 fun:functional=functional
 
 fun:custom*=uninstrumented
 fun:custom*=custom
+
+fun:uninstrumented*=uninstrumented

llvm/trunk/test/Instrumentation/DataFlowSanitizer/uninstrumented_local_functions.ll

+; RUN: opt < %s -dfsan -dfsan-args-abi -dfsan-abilist=%S/Inputs/abilist.txt -S | FileCheck %s
+; RUN: opt < %s -dfsan                 -dfsan-abilist=%S/Inputs/abilist.txt -S | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define internal i8 @uninstrumented_internal_fun(i8 %in) {
+  ret i8 %in
+}
+
+define i8 @call_uninstrumented_internal_fun(i8 %in) {
+  %call = call i8 @uninstrumented_internal_fun(i8 %in)
+  ret i8 %call
+}
+; CHECK: define internal {{(i8|{ i8, i16 })}} @"dfsw$uninstrumented_internal_fun"
+
+define private i8 @uninstrumented_private_fun(i8 %in) {
+  ret i8 %in
+}
+
+define i8 @call_uninstrumented_private_fun(i8 %in) {
+  %call = call i8 @uninstrumented_private_fun(i8 %in)
+  ret i8 %call
+}
+; CHECK: define private {{(i8|{ i8, i16 })}} @"dfsw$uninstrumented_private_fun"