[Flang][WIP/RFC] Enable TSan for Flang #74643
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-flang-driver Author: Joachim (jprotze) ChangesThis patch enables ThreadSanitizer analysis for Fortran codes compiled with Flang. The patch is marked as WIP/RFC since it is at the moment a prove of concept. Open questions from our side:
Full diff: https://github.com/llvm/llvm-project/pull/74643.diff 7 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 0eec2b3526376..2fcc31495cdb5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2163,14 +2163,14 @@ def fmemory_profile_use_EQ : Joined<["-"], "fmemory-profile-use=">,
HelpText<"Use memory profile for profile-guided memory optimization">,
MarshallingInfoString<CodeGenOpts<"MemoryProfileUsePath">>;
+def fsanitize_EQ : CommaJoined<["-"], "fsanitize=">, Group<f_clang_Group>,
+ MetaVarName<"<check>">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>,
+ HelpText<"Turn on runtime checks for various forms of undefined "
+ "or suspicious behavior. See user manual for available checks">;
// Begin sanitizer flags. These should all be core options exposed in all driver
// modes.
let Visibility = [ClangOption, CC1Option, CLOption] in {
-def fsanitize_EQ : CommaJoined<["-"], "fsanitize=">, Group<f_clang_Group>,
- MetaVarName<"<check>">,
- HelpText<"Turn on runtime checks for various forms of undefined "
- "or suspicious behavior. See user manual for available checks">;
def fno_sanitize_EQ : CommaJoined<["-"], "fno-sanitize=">, Group<f_clang_Group>,
Visibility<[ClangOption, CLOption]>;
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 9b21fe952af7a..7d433455f24cc 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -38,6 +38,7 @@ void Flang::addFortranDialectOptions(const ArgList &Args,
options::OPT_fopenmp,
options::OPT_fopenmp_version_EQ,
options::OPT_fopenacc,
+ options::OPT_fsanitize_EQ,
options::OPT_finput_charset_EQ,
options::OPT_fimplicit_none,
options::OPT_fno_implicit_none,
diff --git a/flang/include/flang/Common/Fortran-features.h b/flang/include/flang/Common/Fortran-features.h
index a6b19e9833fc5..d3d4556b9cdbf 100644
--- a/flang/include/flang/Common/Fortran-features.h
+++ b/flang/include/flang/Common/Fortran-features.h
@@ -35,7 +35,7 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
ProgramReturn, ImplicitNoneTypeNever, ImplicitNoneTypeAlways,
ForwardRefImplicitNone, OpenAccessAppend, BOZAsDefaultInteger,
DistinguishableSpecifics, DefaultSave, PointerInSeqType, NonCharacterFormat,
- SaveMainProgram, SaveBigMainProgramVariables,
+ SaveMainProgram, SaveBigMainProgramVariables, TSan,
DistinctArrayConstructorLengths, PPCVector, RelaxedIntentInChecking,
ForwardRefImplicitNoneData, NullActualForAllocatable,
ActualIntegerConvertedToSmallerKind, HollerithOrCharacterAsBOZ,
@@ -65,6 +65,7 @@ class LanguageFeatureControl {
disable_.set(LanguageFeature::OldDebugLines);
disable_.set(LanguageFeature::OpenACC);
disable_.set(LanguageFeature::OpenMP);
+ disable_.set(LanguageFeature::TSan);
disable_.set(LanguageFeature::CUDA); // !@cuf
disable_.set(LanguageFeature::ImplicitNoneTypeNever);
disable_.set(LanguageFeature::ImplicitNoneTypeAlways);
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index c623969a21e5e..6b3c0706c9273 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -862,6 +862,10 @@ static bool parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
res.getFrontendOpts().features.Enable(
Fortran::common::LanguageFeature::OpenACC);
}
+ if (args.hasArg(clang::driver::options::OPT_fsanitize_EQ) && llvm::StringRef(args.getLastArg(clang::driver::options::OPT_fsanitize_EQ)->getValue()) == "thread" ) {
+ res.getFrontendOpts().features.Enable(
+ Fortran::common::LanguageFeature::TSan);
+ }
if (args.hasArg(clang::driver::options::OPT_fopenmp)) {
// By default OpenMP is set to 1.1 version
res.getLangOpts().OpenMPVersion = 11;
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 1be95cc27f42c..423e121cdf3dc 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -67,6 +67,9 @@
#include "llvm/Target/TargetMachine.h"
#include "llvm/TargetParser/TargetParser.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
+#include "llvm/Transforms/Instrumentation.h"
+#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
+
#include <memory>
#include <system_error>
@@ -1073,6 +1076,11 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
else
mpm = pb.buildPerModuleDefaultPipeline(level);
+ if (this->getInstance().getInvocation().getFrontendOpts().features.IsEnabled(
+ Fortran::common::LanguageFeature::TSan)) {
+ mpm.addPass(llvm::ModuleThreadSanitizerPass());
+ mpm.addPass(llvm::createModuleToFunctionPassAdaptor(llvm::ThreadSanitizerPass()));
+ }
if (action == BackendActionTy::Backend_EmitBC)
mpm.addPass(llvm::BitcodeWriterPass(os));
diff --git a/flang/lib/Optimizer/CodeGen/CMakeLists.txt b/flang/lib/Optimizer/CodeGen/CMakeLists.txt
index 0daa97b00dfa0..2d48c873ff4a1 100644
--- a/flang/lib/Optimizer/CodeGen/CMakeLists.txt
+++ b/flang/lib/Optimizer/CodeGen/CMakeLists.txt
@@ -1,3 +1,7 @@
+set(LLVM_LINK_COMPONENTS
+ Instrumentation
+)
+
add_flang_library(FIRCodeGen
BoxedProcedure.cpp
CGOps.cpp
diff --git a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
index 8ee0bca7e354f..bd720634e467e 100644
--- a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -511,7 +511,7 @@ bool ThreadSanitizer::sanitizeFunction(Function &F,
SmallVector<Instruction*, 8> MemIntrinCalls;
bool Res = false;
bool HasCalls = false;
- bool SanitizeFunction = F.hasFnAttribute(Attribute::SanitizeThread);
+ bool SanitizeFunction = F.hasFnAttribute(Attribute::SanitizeThread) || true;
const DataLayout &DL = F.getParent()->getDataLayout();
// Traverse all instructions, collect loads/stores/returns, check for calls.
|
You can test this locally with the following command:git-clang-format --diff 1e1e11a4d7c57028953a23deae622acab5eee9ff 03c22524ecf8851951d37f6282b3cd43edb8607e -- clang/lib/Driver/ToolChains/Flang.cpp flang/include/flang/Common/Fortran-features.h flang/lib/Frontend/CompilerInvocation.cpp flang/lib/Frontend/FrontendActions.cpp llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cppView the diff from clang-format here.diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 6b3c0706c9..ffcd3aefd6 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -862,7 +862,9 @@ static bool parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
res.getFrontendOpts().features.Enable(
Fortran::common::LanguageFeature::OpenACC);
}
- if (args.hasArg(clang::driver::options::OPT_fsanitize_EQ) && llvm::StringRef(args.getLastArg(clang::driver::options::OPT_fsanitize_EQ)->getValue()) == "thread" ) {
+ if (args.hasArg(clang::driver::options::OPT_fsanitize_EQ) &&
+ llvm::StringRef(args.getLastArg(clang::driver::options::OPT_fsanitize_EQ)
+ ->getValue()) == "thread") {
res.getFrontendOpts().features.Enable(
Fortran::common::LanguageFeature::TSan);
}
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 423e121cdf..7abd283114 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -66,9 +66,9 @@
#include "llvm/Support/ToolOutputFile.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/TargetParser/TargetParser.h"
-#include "llvm/Transforms/Utils/ModuleUtils.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <memory>
#include <system_error>
@@ -1079,7 +1079,8 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
if (this->getInstance().getInvocation().getFrontendOpts().features.IsEnabled(
Fortran::common::LanguageFeature::TSan)) {
mpm.addPass(llvm::ModuleThreadSanitizerPass());
- mpm.addPass(llvm::createModuleToFunctionPassAdaptor(llvm::ThreadSanitizerPass()));
+ mpm.addPass(
+ llvm::createModuleToFunctionPassAdaptor(llvm::ThreadSanitizerPass()));
}
if (action == BackendActionTy::Backend_EmitBC)
mpm.addPass(llvm::BitcodeWriterPass(os));
|
jprotze
added a commit
to RWTH-HPC/llvm-project
that referenced
this pull request
Dec 8, 2023
This patch enables ThreadSanitizer analysis for Fortran codes compiled with Flang. The patch is marked as WIP/RFC since it is at the moment a proof of concept. This patch as is cannot be the final solution, as it disables the blacklisting of functions and any no-sanitize function attribute applied in C/C++ Under review as llvm#74643
|
Hi @jprotze , thank you for working on this! One thing that would be very nice to see are tests :) It would really help if we could see how to use and how to test this. Any chance for an end-to-end PoC or a demonstrator? @kiranchandramohan , would an e2e test with OpenMP enabled make sense to exercise this?
These are uncharted waters for me, but it looks like in Clang the sanitizers are added before default optimisation pipleines are built. I would make sure that Flang does the same or justify the rationale for any divergence. As far as the Flang driver is concerned, I think that you have correctly identified the most suitable place. I would probably extract the "sanitzer" logic into a dedicated hook, but that's an implementation detail for later.
How does Clang decide when to add such attributes? Why not replicate that logic? Also, if you are looking for a somewhat relevant/similar pass in Flang, check llvm-project/flang/lib/Optimizer/Transforms/VScaleAttr.cpp Lines 61 to 79 in 435ba72 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch enables ThreadSanitizer analysis for Fortran codes compiled with Flang. The patch is marked as WIP/RFC since it is at the moment a proof of concept.
Open questions from our side: