Skip to content

Commit a12693f

Browse files
committed
Track deletion of merged constants
This fixes an intermittent segfault in the Diffractor test. The error very closely depends on the order of events, but essentially what happens is that a GlobalVariable gets stored into mergedConstants, but then deleted in jl_merge_module when the opaque closure module is merged into its parent. A test case is included, but it is quite specific to current codegen, so it'll probably stop being useful the next time we restructure things. It's also intermittent (though would show up under valgrind/asan). Hopefully, it'll nevertheless be useful.
1 parent 7eb9615 commit a12693f

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

src/codegen.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1750,9 +1750,10 @@ static inline GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G)
17501750

17511751
static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_context, Constant *val, StringRef name, Module &M)
17521752
{
1753-
GlobalVariable *&gv = emission_context.mergedConstants[val];
1753+
WeakVH &vh = emission_context.mergedConstants[val];
17541754
StringRef localname;
17551755
std::string ssno;
1756+
GlobalVariable *gv = cast_or_null<GlobalVariable>((Value*)vh);
17561757
if (gv == nullptr) {
17571758
raw_string_ostream(ssno) << name << emission_context.mergedConstants.size();
17581759
localname = StringRef(ssno);
@@ -1771,6 +1772,7 @@ static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_con
17711772
val,
17721773
localname);
17731774
gv->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
1775+
vh = gv;
17741776
}
17751777
assert(localname == gv->getName());
17761778
assert(val == gv->getInitializer());

src/jitlayers.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ typedef struct _jl_codegen_params_t {
193193
std::map<std::tuple<jl_code_instance_t*,bool>, GlobalVariable*> external_fns;
194194
std::map<jl_datatype_t*, DIType*> ditypes;
195195
std::map<jl_datatype_t*, Type*> llvmtypes;
196-
DenseMap<Constant*, GlobalVariable*> mergedConstants;
196+
DenseMap<Constant*, WeakVH /* GlobalVariable* */> mergedConstants;
197197
// Map from symbol name (in a certain library) to its GV in sysimg and the
198198
// DL handle address in the current session.
199199
StringMap<std::pair<GlobalVariable*,SymMapGV>> libMapGV;

test/opaque_closure.jl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,3 +270,28 @@ let ci = code_typed((x, y...)->(x, y), (Int, Int))[1][1]
270270
@test_throws MethodError oc(1,2,3)
271271
end
272272
end
273+
274+
# Regression test for emission of string constants from opaque closures.
275+
# This test is very specific to the structure of codegen. In particular,
276+
# what this test is trying to force is that the inner `g` opaque closure
277+
# gets merged into the module for `f`, overwriting the `typeassert`
278+
# string constant, but because `f` has its own typeassert after the
279+
# definition of `g`, it gets remembered and a use-after-free is forced
280+
# during the emission of `oc_string_const`.
281+
using Base.Experimental: @opaque
282+
@noinline function oc_string_const(x)
283+
x::Int
284+
end
285+
function oc_string_const2(x)
286+
f = @opaque x->begin
287+
@inline
288+
g = @opaque y->begin
289+
y::Int
290+
end
291+
@noinline g(Base.inferencebarrier(x))
292+
x::Int
293+
end
294+
@noinline f(Base.inferencebarrier(x))
295+
oc_string_const(x)
296+
end
297+
@test_throws TypeError oc_string_const2(1.0)

0 commit comments

Comments
 (0)