Skip to content

Commit f4665b7

Browse files
authored
Singleton hack of fixing static initialisation order fiasco (#154541)
#154528 # Brief Indirect linking of llvm as a shared library is causing a "free() invalid size abortion". In my case, my project depends on google/clspv which in turn pulls `llvm`. Note that the issue does not occur when `clspv` and `llvm` is all statically linked. # Structure of a project which might be causing an error [google/clspv](https://github.com/google/clspv) has been depending on this project (llvm-project), as a static library. My personal project has been depending on [google/clspv](https://github.com/google/clspv) as a shared library. So `MyProject` was linked to shared object `clspv_core.so` which is containing `llvm-project` as its component. # Problem Linking `llvm-project` indirectly to `MyProject` via `clspv_core` was causing the `free() invalid size` abortion. > When library is all statically linked, this problem did not occur. [This issue](#154528) has a full log of the programme running with valgrind. # Reason in my expectation `KnownAssumptionStrings` from [clang/lib/Sema/SemaOpenMP.cpp](https://github.com/llvm/llvm-project/pull/154541/files#diff-032b46da5a8b94f6d8266072e296726c361066e32139024c86dcba5bf64960fc), [llvm/include/llvm/IR/Assumptions.h](https://github.com/llvm/llvm-project/pull/154541/files#diff-ebb09639e5957c2e4d27be9dcb1b1475da67d88db829d24ed8039f351a63ccff), [llvm/lib/IR/Assumptions.cpp](https://github.com/llvm/llvm-project/pull/154541/files#diff-1b490dd29304c875364871e35e1cc8e47bf71898affe3a4dbde6eb91c4016d06) and `FeatureMap` from [llvm/lib/Analysis/MLInlineAdvisor.cpp](https://github.com/llvm/llvm-project/pull/154541/files#diff-26c738eb291410ed83595a4162de617e8cbebddb46331f56d39d193868e29857), [llvm/include/llvm/Analysis/InlineModelFeatureMaps.h](https://github.com/llvm/llvm-project/pull/154541/files#diff-3b5a3359b2a0784186fb3f90dfabf905e8640b6adfd7d2c75259a6835751a6a7) which have been placed on global scope, causing static initialisation order ficasso when indirectly linked by `Myproject`. # Fix trial Changing those global instances I've mentioned ~ `KnownAssumptionStrings` and `FeatureMap` ~ to functions which return a static variable's left value ~ `getKnownAssumptionStrings()`, `getFeatureMap()` ~ has solved my personal problem, so I am pulling a request of it.
1 parent 70ddd83 commit f4665b7

File tree

5 files changed

+34
-25
lines changed

5 files changed

+34
-25
lines changed

clang/lib/Sema/SemaOpenMP.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24834,12 +24834,12 @@ ExprResult SemaOpenMP::ActOnOMPIteratorExpr(Scope *S,
2483424834
/// Check if \p AssumptionStr is a known assumption and warn if not.
2483524835
static void checkOMPAssumeAttr(Sema &S, SourceLocation Loc,
2483624836
StringRef AssumptionStr) {
24837-
if (llvm::KnownAssumptionStrings.count(AssumptionStr))
24837+
if (llvm::getKnownAssumptionStrings().count(AssumptionStr))
2483824838
return;
2483924839

2484024840
unsigned BestEditDistance = 3;
2484124841
StringRef Suggestion;
24842-
for (const auto &KnownAssumptionIt : llvm::KnownAssumptionStrings) {
24842+
for (const auto &KnownAssumptionIt : llvm::getKnownAssumptionStrings()) {
2484324843
unsigned EditDistance =
2484424844
AssumptionStr.edit_distance(KnownAssumptionIt.getKey());
2484524845
if (EditDistance < BestEditDistance) {

llvm/include/llvm/Analysis/InlineModelFeatureMaps.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ inlineCostFeatureToMlFeature(InlineCostFeatureIndex Feature) {
160160
return static_cast<FeatureIndex>(static_cast<size_t>(Feature));
161161
}
162162

163-
LLVM_ABI extern std::vector<TensorSpec> FeatureMap;
163+
LLVM_ABI extern std::vector<TensorSpec> &getFeatureMap();
164164

165165
LLVM_ABI extern const char *const DecisionName;
166166
LLVM_ABI extern const TensorSpec InlineDecisionSpec;

llvm/include/llvm/IR/Assumptions.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,18 @@ constexpr StringRef AssumptionAttrKey = "llvm.assume";
3030

3131
/// A set of known assumption strings that are accepted without warning and
3232
/// which can be recommended as typo correction.
33-
LLVM_ABI extern StringSet<> KnownAssumptionStrings;
33+
LLVM_ABI extern StringSet<> &getKnownAssumptionStrings();
3434

3535
/// Helper that allows to insert a new assumption string in the known assumption
3636
/// set by creating a (static) object.
3737
struct KnownAssumptionString {
3838
KnownAssumptionString(const char *AssumptionStr)
3939
: AssumptionStr(AssumptionStr) {
40-
KnownAssumptionStrings.insert(AssumptionStr);
40+
getKnownAssumptionStrings().insert(AssumptionStr);
4141
}
4242
KnownAssumptionString(StringRef AssumptionStr)
4343
: AssumptionStr(AssumptionStr) {
44-
KnownAssumptionStrings.insert(AssumptionStr);
44+
getKnownAssumptionStrings().insert(AssumptionStr);
4545
}
4646
operator StringRef() const { return AssumptionStr; }
4747

llvm/lib/Analysis/MLInlineAdvisor.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "llvm/Analysis/ProfileSummaryInfo.h"
2828
#include "llvm/Analysis/ReleaseModeModelRunner.h"
2929
#include "llvm/Analysis/TargetTransformInfo.h"
30+
#include "llvm/Analysis/TensorSpec.h"
3031
#include "llvm/IR/Dominators.h"
3132
#include "llvm/IR/InstIterator.h"
3233
#include "llvm/IR/Module.h"
@@ -77,10 +78,10 @@ llvm::getReleaseModeAdvisor(Module &M, ModuleAnalysisManager &MAM,
7778
std::unique_ptr<MLModelRunner> AOTRunner;
7879
if (InteractiveChannelBaseName.empty())
7980
AOTRunner = std::make_unique<ReleaseModeModelRunner<CompiledModelType>>(
80-
M.getContext(), FeatureMap, DecisionName,
81+
M.getContext(), getFeatureMap(), DecisionName,
8182
EmbeddedModelRunnerOptions().setModelSelector(ModelSelector));
8283
else {
83-
auto Features = FeatureMap;
84+
auto Features = getFeatureMap();
8485
if (InteractiveIncludeDefault)
8586
Features.push_back(DefaultDecisionSpec);
8687
AOTRunner = std::make_unique<InteractiveModelRunner>(
@@ -106,8 +107,9 @@ static cl::opt<bool> KeepFPICache(
106107
"For test - keep the ML Inline advisor's FunctionPropertiesInfo cache"),
107108
cl::init(false));
108109

109-
// clang-format off
110-
std::vector<TensorSpec> llvm::FeatureMap{
110+
std::vector<TensorSpec> &llvm::getFeatureMap() {
111+
// clang-format off
112+
static std::vector<TensorSpec> FeatureMap{
111113
#define POPULATE_NAMES(DTYPE, SHAPE, NAME, __) TensorSpec::createSpec<DTYPE>(#NAME, SHAPE),
112114
// InlineCost features - these must come first
113115
INLINE_COST_FEATURE_ITERATOR(POPULATE_NAMES)
@@ -116,7 +118,9 @@ std::vector<TensorSpec> llvm::FeatureMap{
116118
INLINE_FEATURE_ITERATOR(POPULATE_NAMES)
117119
#undef POPULATE_NAMES
118120
};
119-
// clang-format on
121+
// clang-format on
122+
return FeatureMap;
123+
}
120124

121125
const char *const llvm::DecisionName = "inlining_decision";
122126
const TensorSpec llvm::InlineDecisionSpec =
@@ -195,9 +199,9 @@ MLInlineAdvisor::MLInlineAdvisor(
195199
}
196200
// Add the IR2Vec features to the feature map
197201
auto IR2VecDim = IR2VecVocabResult->getDimension();
198-
FeatureMap.push_back(
202+
getFeatureMap().push_back(
199203
TensorSpec::createSpec<float>("callee_embedding", {IR2VecDim}));
200-
FeatureMap.push_back(
204+
getFeatureMap().push_back(
201205
TensorSpec::createSpec<float>("caller_embedding", {IR2VecDim}));
202206
}
203207
}
@@ -471,7 +475,8 @@ std::unique_ptr<InlineAdvice> MLInlineAdvisor::getAdviceImpl(CallBase &CB) {
471475
}
472476
// This one would have been set up to be right at the end.
473477
if (!InteractiveChannelBaseName.empty() && InteractiveIncludeDefault)
474-
*ModelRunner->getTensor<int64_t>(FeatureMap.size()) = GetDefaultAdvice(CB);
478+
*ModelRunner->getTensor<int64_t>(getFeatureMap().size()) =
479+
GetDefaultAdvice(CB);
475480
return getAdviceFromModel(CB, ORE);
476481
}
477482

@@ -549,8 +554,8 @@ void MLInlineAdvice::reportContextForRemark(
549554
DiagnosticInfoOptimizationBase &OR) {
550555
using namespace ore;
551556
OR << NV("Callee", Callee->getName());
552-
for (size_t I = 0; I < FeatureMap.size(); ++I)
553-
OR << NV(FeatureMap[I].name(),
557+
for (size_t I = 0; I < getFeatureMap().size(); ++I)
558+
OR << NV(getFeatureMap()[I].name(),
554559
*getAdvisor()->getModelRunner().getTensor<int64_t>(I));
555560
OR << NV("ShouldInline", isInliningRecommended());
556561
}

llvm/lib/IR/Assumptions.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,16 @@ bool llvm::addAssumptions(CallBase &CB,
101101
return ::addAssumptionsImpl(CB, Assumptions);
102102
}
103103

104-
StringSet<> llvm::KnownAssumptionStrings({
105-
"omp_no_openmp", // OpenMP 5.1
106-
"omp_no_openmp_routines", // OpenMP 5.1
107-
"omp_no_parallelism", // OpenMP 5.1
108-
"omp_no_openmp_constructs", // OpenMP 6.0
109-
"ompx_spmd_amenable", // OpenMPOpt extension
110-
"ompx_no_call_asm", // OpenMPOpt extension
111-
"ompx_aligned_barrier", // OpenMPOpt extension
112-
});
104+
StringSet<> &llvm::getKnownAssumptionStrings() {
105+
static StringSet<> Object({
106+
"omp_no_openmp", // OpenMP 5.1
107+
"omp_no_openmp_routines", // OpenMP 5.1
108+
"omp_no_parallelism", // OpenMP 5.1
109+
"omp_no_openmp_constructs", // OpenMP 6.0
110+
"ompx_spmd_amenable", // OpenMPOpt extension
111+
"ompx_no_call_asm", // OpenMPOpt extension
112+
"ompx_aligned_barrier", // OpenMPOpt extension
113+
});
114+
115+
return Object;
116+
}

0 commit comments

Comments
 (0)