diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -52,6 +52,7 @@ CODEGENOPT(DisableFree , 1, 0) ///< Don't free memory. CODEGENOPT(DiscardValueNames , 1, 0) ///< Discard Value Names from the IR (LLVMContext flag) +CODEGENOPT(DisableNoundefAttrs , 1, 0) ///< Disable emitting `noundef` attributes on IR call arguments and return values CODEGENOPT(DisableGCov , 1, 0) ///< Don't run the GCov pass, for testing. CODEGENOPT(DisableLLVMPasses , 1, 0) ///< Don't run any LLVM IR passes to get ///< the pristine IR generated by the diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3972,6 +3972,8 @@ HelpText<"Include code completion results which require small fix-its.">; def disable_free : Flag<["-"], "disable-free">, HelpText<"Disable freeing of memory on exit">; +def disable_noundef_analysis : Flag<["-"], "disable-noundef-analysis">, Group, + HelpText<"Disable analyzing function argument and return types for mandatory definedness">; def discard_value_names : Flag<["-"], "discard-value-names">, HelpText<"Discard value names in LLVM IR">; def load : Separate<["-"], "load">, MetaVarName<"">, diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -1706,6 +1706,18 @@ FuncAttrs.addAttribute(llvm::Attribute::NoUnwind); } +bool CodeGenModule::MayDropFunctionReturn(const ASTContext &Context, + QualType ReturnType) { + // We can't just discard the return value for a record type with a + // complex destructor or a non-trivially copyable type. + if (const RecordType *RT = + ReturnType.getCanonicalType()->getAs()) { + if (const auto *ClassDecl = dyn_cast(RT->getDecl())) + return ClassDecl->hasTrivialDestructor(); + } + return ReturnType.isTriviallyCopyableType(Context); +} + void CodeGenModule::getDefaultFunctionAttributes(StringRef Name, bool HasOptnone, bool AttrOnCallSite, @@ -1883,6 +1895,54 @@ llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr); } +static bool DetermineNoUndef(QualType QTy, CodeGenTypes &Types, + const llvm::DataLayout &DL, const ABIArgInfo &AI, + bool CheckCoerce = true) { + llvm::Type *Ty = Types.ConvertTypeForMem(QTy); + if (AI.getKind() == ABIArgInfo::Indirect) + return true; + if (AI.getKind() == ABIArgInfo::Extend) + return true; + if (!DL.typeSizeEqualsStoreSize(Ty)) + // TODO: This will result in a modest amount of values not marked noundef + // when they could be. We care about values that *invisibly* contain undef + // bits from the perspective of LLVM IR. + return false; + if (CheckCoerce && AI.canHaveCoerceToType()) { + llvm::Type *CoerceTy = AI.getCoerceToType(); + if (DL.getTypeSizeInBits(CoerceTy) > DL.getTypeSizeInBits(Ty)) + // If we're coercing to a type with a greater size than the canonical one, + // we're introducing new undef bits. + // Coercing to a type of smaller or equal size is ok, as we know that + // there's no internal padding (typeSizeEqualsStoreSize). + return false; + } + if (QTy->isExtIntType()) + return true; + if (QTy->isReferenceType()) + return true; + if (QTy->isNullPtrType()) + return false; + if (QTy->isMemberPointerType()) + // TODO: Some member pointers are `noundef`, but it depends on the ABI. For + // now, never mark them. + return false; + if (QTy->isScalarType()) { + if (const ComplexType *Complex = dyn_cast(QTy)) + return DetermineNoUndef(Complex->getElementType(), Types, DL, AI, false); + return true; + } + if (const VectorType *Vector = dyn_cast(QTy)) + return DetermineNoUndef(Vector->getElementType(), Types, DL, AI, false); + if (const MatrixType *Matrix = dyn_cast(QTy)) + return DetermineNoUndef(Matrix->getElementType(), Types, DL, AI, false); + if (const ArrayType *Array = dyn_cast(QTy)) + return DetermineNoUndef(Array->getElementType(), Types, DL, AI, false); + + // TODO: Some structs may be `noundef`, in specific situations. + return false; +} + /// Construct the IR attribute list of a function or call. /// /// When adding an attribute, please consider where it should be handled: @@ -2082,6 +2142,34 @@ QualType RetTy = FI.getReturnType(); const ABIArgInfo &RetAI = FI.getReturnInfo(); + const llvm::DataLayout &DL = getDataLayout(); + + // C++ explicitly makes returning undefined values UB. C's rule only applies + // to used values, so we never mark them noundef for now. + bool HasStrictReturn = getLangOpts().CPlusPlus; + if (TargetDecl) { + if (const FunctionDecl *FDecl = dyn_cast(TargetDecl)) + HasStrictReturn &= !FDecl->isExternC(); + else if (const VarDecl *VDecl = dyn_cast(TargetDecl)) + // Function pointer + HasStrictReturn &= !VDecl->isExternC(); + } + + // We don't want to be too aggressive with the return checking, unless + // it's explicit in the code opts or we're using an appropriate sanitizer. + // Try to respect what the programmer intended. + HasStrictReturn &= getCodeGenOpts().StrictReturn || + !MayDropFunctionReturn(getContext(), RetTy) || + getLangOpts().Sanitize.has(SanitizerKind::Memory) || + getLangOpts().Sanitize.has(SanitizerKind::Return); + + // Determine if the return type could be partially undef + if (!CodeGenOpts.DisableNoundefAttrs && HasStrictReturn) { + if (!RetTy->isVoidType() && RetAI.getKind() != ABIArgInfo::Indirect && + DetermineNoUndef(RetTy, getTypes(), DL, RetAI)) + RetAttrs.addAttribute(llvm::Attribute::NoUndef); + } + switch (RetAI.getKind()) { case ABIArgInfo::Extend: if (RetAI.isSignExt()) @@ -2168,6 +2256,11 @@ } } + // Decide whether the argument we're handling could be partially undef + bool ArgNoUndef = DetermineNoUndef(ParamType, getTypes(), DL, AI); + if (!CodeGenOpts.DisableNoundefAttrs && ArgNoUndef) + Attrs.addAttribute(llvm::Attribute::NoUndef); + // 'restrict' -> 'noalias' is done in EmitFunctionProlog when we // have the corresponding parameter variable. It doesn't make // sense to do it here because parameters are so messed up. diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1230,19 +1230,6 @@ return ResTy; } -static bool -shouldUseUndefinedBehaviorReturnOptimization(const FunctionDecl *FD, - const ASTContext &Context) { - QualType T = FD->getReturnType(); - // Avoid the optimization for functions that return a record type with a - // trivial destructor or another trivially copyable type. - if (const RecordType *RT = T.getCanonicalType()->getAs()) { - if (const auto *ClassDecl = dyn_cast(RT->getDecl())) - return !ClassDecl->hasTrivialDestructor(); - } - return !T.isTriviallyCopyableType(Context); -} - void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, const CGFunctionInfo &FnInfo) { const FunctionDecl *FD = cast(GD.getDecl()); @@ -1323,7 +1310,7 @@ !FD->getReturnType()->isVoidType() && Builder.GetInsertBlock()) { bool ShouldEmitUnreachable = CGM.getCodeGenOpts().StrictReturn || - shouldUseUndefinedBehaviorReturnOptimization(FD, getContext()); + !CGM.MayDropFunctionReturn(FD->getASTContext(), FD->getReturnType()); if (SanOpts.has(SanitizerKind::Return)) { SanitizerScope SanScope(this); llvm::Value *IsFalse = Builder.getFalse(); diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1356,6 +1356,10 @@ void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD, llvm::Function *F); + /// Whether this function's return type has no side effects, and thus may + /// be trivially discarded if it is unused. + bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType); + /// Returns whether this module needs the "all-vtables" type identifier. bool NeedAllVtablesTypeId() const; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -924,6 +924,7 @@ Opts.DisableFree = Args.hasArg(OPT_disable_free); Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names); + Opts.DisableNoundefAttrs = Args.hasArg(OPT_disable_noundef_analysis); Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls); Opts.NoEscapingBlockTailCalls = Args.hasArg(OPT_fno_escaping_block_tail_calls); diff --git a/clang/test/CodeGen/attr-noundef.cpp b/clang/test/CodeGen/attr-noundef.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGen/attr-noundef.cpp @@ -0,0 +1,163 @@ +// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL +// RUN: %clang_bin -cc1 -triple x86_64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-INTEL +// RUN: %clang_bin -cc1 -triple aarch64-gnu-linux -x c++ -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-AARCH + +//************ Passing structs by value +// TODO: No structs may currently be marked noundef + +namespace check_structs { +struct Trivial { + int a; +}; +Trivial ret_trivial() { return {}; } +void pass_trivial(Trivial e) {} +// CHECK-INTEL: define i32 @{{.*}}ret_trivial +// CHECK-AARCH: define i64 @{{.*}}ret_trivial +// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 % +// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 % + +struct NoCopy { + int a; + NoCopy(NoCopy &) = delete; +}; +NoCopy ret_nocopy() { return {}; } +void pass_nocopy(NoCopy e) {} +// CHECK: define {{(dso_local)?}}void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 % +// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef % + +struct Huge { + int a[1024]; +}; +Huge ret_huge() { return {}; } +void pass_huge(Huge h) {} +// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 % +// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef +} // namespace check_structs + +//************ Passing unions by value +// No unions may be marked noundef + +namespace check_unions { +union Trivial { + int a; +}; +Trivial ret_trivial() { return {}; } +void pass_trivial(Trivial e) {} +// CHECK-INTEL: define i32 @{{.*}}ret_trivial +// CHECK-AARCH: define i64 @{{.*}}ret_trivial +// CHECK-INTEL: define void @{{.*}}pass_trivial{{.*}}(i32 % +// CHECK-AARCH: define void @{{.*}}pass_trivial{{.*}}(i64 % + +union NoCopy { + int a; + NoCopy(NoCopy &) = delete; +}; +NoCopy ret_nocopy() { return {}; } +void pass_nocopy(NoCopy e) {} +// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 % +// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef % +} // namespace check_unions + +//************ Passing `this` pointers +// `this` pointer must always be defined + +namespace check_this { +struct Object { + int data[]; + + Object() { + this->data[0] = 0; + } + int getData() { + return this->data[0]; + } + Object *getThis() { + return this; + } +}; + +void use_object() { + Object obj; + obj.getData(); + obj.getThis(); +} +// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef % +// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef % +// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef % +} // namespace check_this + +//************ Passing vector types + +namespace check_vecs { +typedef int __attribute__((vector_size(12))) i32x3; +i32x3 ret_vec() { + return {}; +} +void pass_vec(i32x3 v) { +} + +// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}() +// CHECK-INTEL: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef % +// CHECK-AARCH: define void @{{.*}}pass_vec{{.*}}(<4 x i32> % +} // namespace check_vecs + +//************ Passing exotic types +// Function/Array pointers, Function member / Data member pointers, nullptr_t, ExtInt types + +namespace check_exotic { +struct Object { + int mfunc(); + int mdata; +}; +typedef int Object::*mdptr; +typedef int (Object::*mfptr)(); +typedef decltype(nullptr) nullptr_t; +typedef int (*arrptr)[32]; +typedef int (*fnptr)(int); + +arrptr ret_arrptr() { + return nullptr; +} +fnptr ret_fnptr() { + return nullptr; +} +mdptr ret_mdptr() { + return nullptr; +} +mfptr ret_mfptr() { + return nullptr; +} +nullptr_t ret_npt() { + return nullptr; +} +void pass_npt(nullptr_t t) { +} +_ExtInt(3) ret_extint() { + return 0; +} +void pass_extint(_ExtInt(3) e) { +} +void pass_large_extint(_ExtInt(127) e) { +} + +// Pointers to arrays/functions are always noundef +// CHECK: define noundef [32 x i32]* @{{.*}}ret_arrptr{{.*}}() +// CHECK: define noundef i32 (i32)* @{{.*}}ret_fnptr{{.*}}() + +// Pointers to members are never noundef +// CHECK: define i64 @{{.*}}ret_mdptr{{.*}}() +// CHECK-INTEL: define { i64, i64 } @{{.*}}ret_mfptr{{.*}}() +// CHECK-AARCH: define [2 x i64] @{{.*}}ret_mfptr{{.*}}() + +// nullptr_t is never noundef +// CHECK: define i8* @{{.*}}ret_npt{{.*}}() +// CHECK: define void @{{.*}}pass_npt{{.*}}(i8* % + +// TODO: for now, ExtInt is only noundef if it is sign/zero-extended +// CHECK-INTEL: define noundef signext i3 @{{.*}}ret_extint{{.*}}() +// CHECK-AARCH: define i3 @{{.*}}ret_extint{{.*}}() +// CHECK-INTEL: define void @{{.*}}pass_extint{{.*}}(i3 noundef signext % +// CHECK-AARCH: define void @{{.*}}pass_extint{{.*}}(i3 % +// CHECK-INTEL: define void @{{.*}}pass_large_extint{{.*}}(i64 %{{.*}}, i64 % +// CHECK-AARCH: define void @{{.*}}pass_large_extint{{.*}}(i127 % +} // namespace check_exotic diff --git a/clang/test/CodeGen/indirect-noundef.cpp b/clang/test/CodeGen/indirect-noundef.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGen/indirect-noundef.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_bin -cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s + +union u1 { + int val; +}; + +// CHECK: @indirect_callee_int_ptr = global i32 (i32)* +int (*indirect_callee_int_ptr)(int); +// CHECK: @indirect_callee_union_ptr = global i32 (i32)* +union u1 (*indirect_callee_union_ptr)(union u1); + +// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef % +int indirect_callee_int(int a) { return a; } +// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 % +union u1 indirect_callee_union(union u1 a) { + return a; +} + +int main() { + // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0) + indirect_callee_int(0); + // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 % + indirect_callee_union((union u1){0}); + + indirect_callee_int_ptr = indirect_callee_int; + indirect_callee_union_ptr = indirect_callee_union; + + // CHECK: call noundef i32 %{{.*}}(i32 noundef 0) + indirect_callee_int_ptr(0); + // CHECK: call i32 %{{.*}}(i32 % + indirect_callee_union_ptr((union u1){}); + + return 0; +} diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -41,7 +41,8 @@ llvm_config.use_default_substitutions() -llvm_config.use_clang() +llvm_config.use_clang(cc_additional_flags=['-Xclang -disable-noundef-analysis'], + cc1_additional_flags=['-disable-noundef-analysis']) config.substitutions.append( ('%src_include_dir', config.clang_src_dir + '/include')) diff --git a/llvm/utils/update_cc_test_checks.py b/llvm/utils/update_cc_test_checks.py --- a/llvm/utils/update_cc_test_checks.py +++ b/llvm/utils/update_cc_test_checks.py @@ -28,9 +28,9 @@ from UpdateTestChecks import common SUBST = { - '%clang': [], - '%clang_cc1': ['-cc1'], - '%clangxx': ['--driver-mode=g++'], + '%clang': ['-Xclang -disable-noundef-analysis'], + '%clang_cc1': ['-cc1', '-disable-noundef-analysis'], + '%clangxx': ['--driver-mode=g++', '-disable-noundef-analysis'], } def get_line2spell_and_mangled(args, clang_args): @@ -161,8 +161,8 @@ try: builtin_include_dir = subprocess.check_output( [args.clang, '-print-file-name=include']).decode().strip() - SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir, - '-nostdsysteminc'] + SUBST['%clang_cc1'] += ['-internal-isystem', builtin_include_dir, + '-nostdsysteminc'] except subprocess.CalledProcessError: common.warn('Could not determine clang builtins directory, some tests ' 'might not update correctly.')