Author: Fangrui Song Date: 2020-05-10T12:37:44-07:00 New Revision: 13a633b438b6500ecad9e4f936ebadf3411d0f44
URL: https://github.com/llvm/llvm-project/commit/13a633b438b6500ecad9e4f936ebadf3411d0f44 DIFF: https://github.com/llvm/llvm-project/commit/13a633b438b6500ecad9e4f936ebadf3411d0f44.diff LOG: [gcov] Delete CC1 option -coverage-no-function-names-in-data rL144865 incorrectly wrote function names for GCOV_TAG_FUNCTION (this might be part of the reasons the header says "We emit files in a corrupt version of GCOV's "gcda" file format"). rL176173 and rL177475 realized the problem and introduced -coverage-no-function-names-in-data to work around the issue. (However, the description is wrong. libgcov never writes function names, even before GCC 4.2). In reality, the linker command line has to look like: clang --coverage -Xclang -coverage-version='407*' -Xclang -coverage-cfg-checksum -Xclang -coverage-no-function-names-in-data Failing to pass -coverage-no-function-names-in-data can make gcov 4.7~7 either produce wrong results (for one gcov-4.9 program, I see "No executable lines") or segfault (gcov-7). (gcov-8 uses an incompatible format.) This patch deletes -coverage-no-function-names-in-data and the related function names support from libclang_rt.profile Added: Modified: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/CC1Options.td clang/lib/CodeGen/BackendUtil.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/test/CodeGen/code-coverage.c compiler-rt/lib/profile/GCDAProfiling.c llvm/include/llvm/Transforms/Instrumentation.h llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp llvm/test/Transforms/GCOVProfiling/function-numbering.ll Removed: ################################################################################ diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 5c7fbf43ce46..caf652ad7bbb 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -40,7 +40,6 @@ CODEGENOPT(Backchain , 1, 0) ///< -mbackchain CODEGENOPT(ControlFlowGuardNoChecks , 1, 0) ///< -cfguard-no-checks CODEGENOPT(ControlFlowGuard , 1, 0) ///< -cfguard CODEGENOPT(CoverageExtraChecksum, 1, 0) ///< Whether we need a second checksum for functions in GCNO files. -CODEGENOPT(CoverageNoFunctionNamesInData, 1, 0) ///< Do not include function names in GCDA files. CODEGENOPT(CoverageExitBlockBeforeBody, 1, 0) ///< Whether to emit the exit block before the body blocks in GCNO files. CODEGENOPT(CXAAtExit , 1, 1) ///< Use __cxa_atexit for calling destructors. CODEGENOPT(RegisterGlobalDtorsWithAtExit, 1, 1) ///< Use atexit or __cxa_atexit to register global destructors. diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index e7912dd27ea6..7d04d80eda81 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -264,8 +264,6 @@ def coverage_notes_file_EQ : Joined<["-"], "coverage-notes-file=">, Alias<coverage_notes_file>; def coverage_cfg_checksum : Flag<["-"], "coverage-cfg-checksum">, HelpText<"Emit CFG checksum for functions in .gcno files.">; -def coverage_no_function_names_in_data : Flag<["-"], "coverage-no-function-names-in-data">, - HelpText<"Emit function names in .gcda files.">; def coverage_exit_block_before_body : Flag<["-"], "coverage-exit-block-before-body">, HelpText<"Emit the exit block before the body blocks in .gcno files.">; def coverage_version_EQ : Joined<["-"], "coverage-version=">, diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 3a42ca641e96..0c31bc026ab2 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -532,7 +532,6 @@ static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts) { llvm::copy(CodeGenOpts.CoverageVersion, std::begin(Options.Version)); Options.UseCfgChecksum = CodeGenOpts.CoverageExtraChecksum; Options.NoRedZone = CodeGenOpts.DisableRedZone; - Options.FunctionNamesInData = !CodeGenOpts.CoverageNoFunctionNamesInData; Options.Filter = CodeGenOpts.ProfileFilterFiles; Options.Exclude = CodeGenOpts.ProfileExcludeFiles; Options.ExitBlockBeforeBody = CodeGenOpts.CoverageExitBlockBeforeBody; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 01f06731dd6e..6a358f235d8a 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1018,8 +1018,6 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, Opts.CoverageNotesFile = std::string(Args.getLastArgValue(OPT_coverage_notes_file)); Opts.CoverageExtraChecksum = Args.hasArg(OPT_coverage_cfg_checksum); - Opts.CoverageNoFunctionNamesInData = - Args.hasArg(OPT_coverage_no_function_names_in_data); Opts.ProfileFilterFiles = std::string(Args.getLastArgValue(OPT_fprofile_filter_files_EQ)); Opts.ProfileExcludeFiles = diff --git a/clang/test/CodeGen/code-coverage.c b/clang/test/CodeGen/code-coverage.c index 98153933c918..c93624a2878d 100644 --- a/clang/test/CodeGen/code-coverage.c +++ b/clang/test/CodeGen/code-coverage.c @@ -1,5 +1,4 @@ // RUN: %clang_cc1 -emit-llvm -disable-red-zone -femit-coverage-data %s -o - | FileCheck %s -// RUN: %clang_cc1 -emit-llvm -disable-red-zone -femit-coverage-data -coverage-no-function-names-in-data %s -o - | FileCheck %s --check-prefix WITHOUTNAMES // RUN: %clang_cc1 -emit-llvm -disable-red-zone -femit-coverage-data -coverage-notes-file=aaa.gcno -coverage-data-file=bbb.gcda -dwarf-column-info -debug-info-kind=limited -dwarf-version=4 %s -o - | FileCheck %s --check-prefix GCOV_FILE_INFO // RUN: %clang_cc1 -emit-llvm-bc -o /dev/null -fexperimental-new-pass-manager -fdebug-pass-manager -femit-coverage-data %s 2>&1 | FileCheck --check-prefix=NEWPM %s @@ -27,16 +26,8 @@ int test2(int b) { return b * 2; } -// Inside function emission data structure, check that -// -coverage-no-function-names-in-data uses null as the function name. -// CHECK: @__llvm_internal_gcov_emit_function_args.0 = internal unnamed_addr constant -// CHECK-SAME: { i32 {{[0-9]+}}, i8* getelementptr inbounds ({{[^,]*}}, {{[^,]*}}* @ -// CHECK-SAME: { i32 {{[0-9]+}}, i8* getelementptr inbounds ({{[^,]*}}, {{[^,]*}}* @ -// WITHOUTNAMES: @__llvm_internal_gcov_emit_function_args.0 = internal unnamed_addr constant -// WITHOUTNAMES-NOT: getelementptr inbounds ({{.*}}@ -// WITHOUTNAMES-SAME: zeroinitializer, -// WITHOUTNAMES-NOT: getelementptr inbounds ({{.*}}@ -// WITHOUTNAMES-SAME: { i32 {{[0-9]+}}, i8* null, +// CHECK: @__llvm_internal_gcov_emit_function_args.0 = internal unnamed_addr constant [2 x %0] +// CHECK-SAME: [%0 zeroinitializer, %0 { i32 1, i32 0, i8 0, i32 0 }] // Check that the noredzone flag is set on the generated functions. diff --git a/compiler-rt/lib/profile/GCDAProfiling.c b/compiler-rt/lib/profile/GCDAProfiling.c index d9e1fb3cdc4e..401969b3e779 100644 --- a/compiler-rt/lib/profile/GCDAProfiling.c +++ b/compiler-rt/lib/profile/GCDAProfiling.c @@ -203,7 +203,12 @@ static uint32_t length_of_string(const char *s) { return (strlen(s) / 4) + 1; } -static void write_string(const char *s) { +// Remove when we support libgcov 9 current_working_directory. +#if !defined(_MSC_VER) && defined(__clang__) +__attribute__((unused)) +#endif +static void +write_string(const char *s) { uint32_t len = length_of_string(s); write_32bit_value(len); write_bytes(s, strlen(s)); @@ -445,30 +450,25 @@ void llvm_gcda_increment_indirect_counter(uint32_t *predecessor, } COMPILER_RT_VISIBILITY -void llvm_gcda_emit_function(uint32_t ident, const char *function_name, - uint32_t func_checksum, uint8_t use_extra_checksum, +void llvm_gcda_emit_function(uint32_t ident, uint32_t func_checksum, + uint8_t use_extra_checksum, uint32_t cfg_checksum) { uint32_t len = 2; if (use_extra_checksum) len++; #ifdef DEBUG_GCDAPROFILING - fprintf(stderr, "llvmgcda: function id=0x%08x name=%s\n", ident, - function_name ? function_name : "NULL"); + fprintf(stderr, "llvmgcda: function id=0x%08x\n", ident); #endif if (!output_file) return; /* function tag */ write_bytes("\0\0\0\1", 4); - if (function_name) - len += 1 + length_of_string(function_name); write_32bit_value(len); write_32bit_value(ident); write_32bit_value(func_checksum); if (use_extra_checksum) write_32bit_value(cfg_checksum); - if (function_name) - write_string(function_name); } COMPILER_RT_VISIBILITY diff --git a/llvm/include/llvm/Transforms/Instrumentation.h b/llvm/include/llvm/Transforms/Instrumentation.h index 584c65403b1b..bedd6f18f338 100644 --- a/llvm/include/llvm/Transforms/Instrumentation.h +++ b/llvm/include/llvm/Transforms/Instrumentation.h @@ -70,10 +70,6 @@ struct GCOVOptions { // Add the 'noredzone' attribute to added runtime library calls. bool NoRedZone; - // Emit the name of the function in the .gcda files. This is redundant, as - // the function identifier can be used to find the name from the .gcno file. - bool FunctionNamesInData; - // Emit the exit block immediately after the start block, rather than after // all of the function body's blocks. bool ExitBlockBeforeBody; diff --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp index 971035b3ccca..0df316e5cdf8 100644 --- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp @@ -61,7 +61,6 @@ GCOVOptions GCOVOptions::getDefault() { Options.EmitData = true; Options.UseCfgChecksum = false; Options.NoRedZone = false; - Options.FunctionNamesInData = true; Options.ExitBlockBeforeBody = DefaultExitBlockBeforeBody; if (DefaultGCOVVersion.size() != 4) { @@ -945,7 +944,6 @@ FunctionCallee GCOVProfiler::getStartFileFunc(const TargetLibraryInfo *TLI) { FunctionCallee GCOVProfiler::getEmitFunctionFunc(const TargetLibraryInfo *TLI) { Type *Args[] = { Type::getInt32Ty(*Ctx), // uint32_t ident - Type::getInt8PtrTy(*Ctx), // const char *function_name Type::getInt32Ty(*Ctx), // uint32_t func_checksum Type::getInt8Ty(*Ctx), // uint8_t use_extra_checksum Type::getInt32Ty(*Ctx), // uint32_t cfg_checksum @@ -1016,9 +1014,9 @@ Function *GCOVProfiler::insertCounterWriteout( // walk to write out everything. StructType *StartFileCallArgsTy = StructType::create( {Builder.getInt8PtrTy(), Builder.getInt8PtrTy(), Builder.getInt32Ty()}); - StructType *EmitFunctionCallArgsTy = StructType::create( - {Builder.getInt32Ty(), Builder.getInt8PtrTy(), Builder.getInt32Ty(), - Builder.getInt8Ty(), Builder.getInt32Ty()}); + StructType *EmitFunctionCallArgsTy = + StructType::create({Builder.getInt32Ty(), Builder.getInt32Ty(), + Builder.getInt8Ty(), Builder.getInt32Ty()}); StructType *EmitArcsCallArgsTy = StructType::create( {Builder.getInt32Ty(), Builder.getInt64Ty()->getPointerTo()}); StructType *FileInfoTy = @@ -1048,14 +1046,10 @@ Function *GCOVProfiler::insertCounterWriteout( SmallVector<Constant *, 8> EmitFunctionCallArgsArray; SmallVector<Constant *, 8> EmitArcsCallArgsArray; for (int j : llvm::seq<int>(0, CountersBySP.size())) { - auto *SP = cast_or_null<DISubprogram>(CountersBySP[j].second); uint32_t FuncChecksum = Funcs.empty() ? 0 : Funcs[j]->getFuncChecksum(); EmitFunctionCallArgsArray.push_back(ConstantStruct::get( EmitFunctionCallArgsTy, {Builder.getInt32(j), - Options.FunctionNamesInData - ? Builder.CreateGlobalStringPtr(getFunctionName(SP)) - : Constant::getNullValue(Builder.getInt8PtrTy()), Builder.getInt32(FuncChecksum), Builder.getInt8(Options.UseCfgChecksum), Builder.getInt32(CfgChecksum)})); @@ -1188,17 +1182,14 @@ Function *GCOVProfiler::insertCounterWriteout( Builder.CreateStructGEP(EmitFunctionCallArgsTy, EmitFunctionCallArgsPtr, 2)), Builder.CreateLoad(EmitFunctionCallArgsTy->getElementType(3), - Builder.CreateStructGEP(EmitFunctionCallArgsTy, - EmitFunctionCallArgsPtr, 3)), - Builder.CreateLoad(EmitFunctionCallArgsTy->getElementType(4), Builder.CreateStructGEP(EmitFunctionCallArgsTy, EmitFunctionCallArgsPtr, - 4))}); + 3))}); if (auto AK = TLI->getExtAttrForI32Param(false)) { EmitFunctionCall->addParamAttr(0, AK); + EmitFunctionCall->addParamAttr(1, AK); EmitFunctionCall->addParamAttr(2, AK); EmitFunctionCall->addParamAttr(3, AK); - EmitFunctionCall->addParamAttr(4, AK); } auto *EmitArcsCallArgsPtr = Builder.CreateInBoundsGEP(EmitArcsCallArgsTy, EmitArcsCallArgsArray, JV); diff --git a/llvm/test/Transforms/GCOVProfiling/function-numbering.ll b/llvm/test/Transforms/GCOVProfiling/function-numbering.ll index f9a95fd1c25a..dfaacc8d772e 100644 --- a/llvm/test/Transforms/GCOVProfiling/function-numbering.ll +++ b/llvm/test/Transforms/GCOVProfiling/function-numbering.ll @@ -16,12 +16,9 @@ target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-apple-macosx10.10.0" -; GCDA: @[[FOO:[0-9]+]] = private unnamed_addr constant [4 x i8] c"foo\00" -; GCDA-NOT: @{{[0-9]+}} = private unnamed_addr constant .* c"bar\00" -; GCDA: @[[BAZ:[0-9]+]] = private unnamed_addr constant [4 x i8] c"baz\00" ; GCDA: @__llvm_internal_gcov_emit_function_args.0 = internal unnamed_addr constant -; GCDA-SAME: { i32 0, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[FOO]] -; GCDA-SAME: { i32 1, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[BAZ]] +; GCDA-SAME: { i32 0, +; GCDA-SAME: { i32 1, ; ; GCDA-LABEL: define internal void @__llvm_gcov_writeout() {{.*}} { ; GCDA-NEXT: entry: @@ -53,18 +50,15 @@ target triple = "x86_64-apple-macosx10.10.0" ; GCDA-NEXT: %[[EMIT_FUN_ARG_0_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_FUN_ARGS]], i32 0, i32 0 ; GCDA-NEXT: %[[EMIT_FUN_ARG_0:.*]] = load i32, i32* %[[EMIT_FUN_ARG_0_PTR]] ; GCDA-NEXT: %[[EMIT_FUN_ARG_1_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_FUN_ARGS]], i32 0, i32 1 -; GCDA-NEXT: %[[EMIT_FUN_ARG_1:.*]] = load i8*, i8** %[[EMIT_FUN_ARG_1_PTR]] +; GCDA-NEXT: %[[EMIT_FUN_ARG_1:.*]] = load i32, i32* %[[EMIT_FUN_ARG_1_PTR]] ; GCDA-NEXT: %[[EMIT_FUN_ARG_2_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_FUN_ARGS]], i32 0, i32 2 -; GCDA-NEXT: %[[EMIT_FUN_ARG_2:.*]] = load i32, i32* %[[EMIT_FUN_ARG_2_PTR]] +; GCDA-NEXT: %[[EMIT_FUN_ARG_2:.*]] = load i8, i8* %[[EMIT_FUN_ARG_2_PTR]] ; GCDA-NEXT: %[[EMIT_FUN_ARG_3_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_FUN_ARGS]], i32 0, i32 3 -; GCDA-NEXT: %[[EMIT_FUN_ARG_3:.*]] = load i8, i8* %[[EMIT_FUN_ARG_3_PTR]] -; GCDA-NEXT: %[[EMIT_FUN_ARG_4_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_FUN_ARGS]], i32 0, i32 4 -; GCDA-NEXT: %[[EMIT_FUN_ARG_4:.*]] = load i32, i32* %[[EMIT_FUN_ARG_4_PTR]] +; GCDA-NEXT: %[[EMIT_FUN_ARG_3:.*]] = load i32, i32* %[[EMIT_FUN_ARG_3_PTR]] ; GCDA-NEXT: call void @llvm_gcda_emit_function(i32 %[[EMIT_FUN_ARG_0]], -; GCDA-SAME: i8* %[[EMIT_FUN_ARG_1]], -; GCDA-SAME: i32 %[[EMIT_FUN_ARG_2]], -; GCDA-SAME: i8 %[[EMIT_FUN_ARG_3]], -; GCDA-SAME: i32 %[[EMIT_FUN_ARG_4]]) +; GCDA-SAME: i32 %[[EMIT_FUN_ARG_1]], +; GCDA-SAME: i8 %[[EMIT_FUN_ARG_2]], +; GCDA-SAME: i32 %[[EMIT_FUN_ARG_3]]) ; GCDA-NEXT: %[[EMIT_ARCS_ARGS:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_ARCS_ARGS_ARRAY]], i32 %[[JV]] ; GCDA-NEXT: %[[EMIT_ARCS_ARG_0_PTR:.*]] = getelementptr inbounds {{.*}}, {{.*}}* %[[EMIT_ARCS_ARGS]], i32 0, i32 0 ; GCDA-NEXT: %[[EMIT_ARCS_ARG_0:.*]] = load i32, i32* %[[EMIT_ARCS_ARG_0_PTR]] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
