Skip to content

Commit e0fc85e

Browse files
committed
[JITLink] Fix LinkGraph::makeAbsolute, add unit test.
makeAbsolute was not updating the symbol address when applied to external symbols. This commit adds a unit test for makeAbsolute, and updates the makeExternal unit test to check that makeExternal works correctly for absolute symbols.
1 parent 00da9e9 commit e0fc85e

File tree

2 files changed

+98
-5
lines changed

2 files changed

+98
-5
lines changed

llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,9 @@ class LinkGraph {
12061206
"Sym is not in the absolute symbols set");
12071207
assert(Sym.getOffset() == 0 && "Absolute not at offset 0");
12081208
AbsoluteSymbols.erase(&Sym);
1209-
Sym.getAddressable().setAbsolute(false);
1209+
auto &A = Sym.getAddressable();
1210+
A.setAbsolute(false);
1211+
A.setAddress(orc::ExecutorAddr());
12101212
} else {
12111213
assert(Sym.isDefined() && "Sym is not a defined symbol");
12121214
Section &Sec = Sym.getBlock().getSection();
@@ -1231,7 +1233,9 @@ class LinkGraph {
12311233
"Sym is not in the absolute symbols set");
12321234
assert(Sym.getOffset() == 0 && "External is not at offset 0");
12331235
ExternalSymbols.erase(&Sym);
1234-
Sym.getAddressable().setAbsolute(true);
1236+
auto &A = Sym.getAddressable();
1237+
A.setAbsolute(true);
1238+
A.setAddress(Address);
12351239
Sym.setScope(Scope::Local);
12361240
} else {
12371241
assert(Sym.isDefined() && "Sym is not a defined symbol");

llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ TEST(LinkGraphTest, ContentAccessAndUpdate) {
209209
}
210210

211211
TEST(LinkGraphTest, MakeExternal) {
212-
// Check that we can make a defined symbol external.
212+
// Check that we can make defined and absolute symbols external.
213213
LinkGraph G("foo", Triple("x86_64-apple-darwin"), 8, support::little,
214214
getGenericEdgeKindName);
215215
auto &Sec = G.createSection("__data", MemProt::Read | MemProt::Write);
@@ -237,22 +237,111 @@ TEST(LinkGraphTest, MakeExternal) {
237237
0U)
238238
<< "Unexpected number of external symbols";
239239

240-
// Make S1 external, confirm that the its flags are updated and that it is
241-
// moved from the defined symbols to the externals list.
240+
// Add an absolute symbol.
241+
auto &S2 = G.addAbsoluteSymbol("S2", orc::ExecutorAddr(0x2000), 0,
242+
Linkage::Strong, Scope::Default, true);
243+
244+
EXPECT_TRUE(S2.isAbsolute()) << "Symbol should be absolute";
245+
EXPECT_EQ(
246+
std::distance(G.absolute_symbols().begin(), G.absolute_symbols().end()),
247+
1U)
248+
<< "Unexpected number of symbols";
249+
250+
// Make S1 and S2 external, confirm that the its flags are updated and that it
251+
// is moved from the defined/absolute symbols lists to the externals list.
242252
G.makeExternal(S1);
253+
G.makeExternal(S2);
243254

244255
EXPECT_FALSE(S1.isDefined()) << "Symbol should not be defined";
245256
EXPECT_TRUE(S1.isExternal()) << "Symbol should be external";
246257
EXPECT_FALSE(S1.isAbsolute()) << "Symbol should not be absolute";
258+
EXPECT_FALSE(S2.isDefined()) << "Symbol should not be defined";
259+
EXPECT_TRUE(S2.isExternal()) << "Symbol should be external";
260+
EXPECT_FALSE(S2.isAbsolute()) << "Symbol should not be absolute";
261+
247262
EXPECT_EQ(S1.getAddress(), orc::ExecutorAddr())
248263
<< "Unexpected symbol address";
264+
EXPECT_EQ(S2.getAddress(), orc::ExecutorAddr())
265+
<< "Unexpected symbol address";
249266

250267
EXPECT_EQ(
251268
std::distance(G.defined_symbols().begin(), G.defined_symbols().end()), 0U)
252269
<< "Unexpected number of defined symbols";
270+
EXPECT_EQ(
271+
std::distance(G.external_symbols().begin(), G.external_symbols().end()),
272+
2U)
273+
<< "Unexpected number of external symbols";
274+
EXPECT_EQ(
275+
std::distance(G.absolute_symbols().begin(), G.absolute_symbols().end()),
276+
0U)
277+
<< "Unexpected number of external symbols";
278+
}
279+
280+
TEST(LinkGraphTest, MakeAbsolute) {
281+
// Check that we can make defined and external symbols absolute.
282+
LinkGraph G("foo", Triple("x86_64-apple-darwin"), 8, support::little,
283+
getGenericEdgeKindName);
284+
auto &Sec = G.createSection("__data", MemProt::Read | MemProt::Write);
285+
286+
// Create an initial block.
287+
auto &B1 =
288+
G.createContentBlock(Sec, BlockContent, orc::ExecutorAddr(0x1000), 8, 0);
289+
290+
// Add a symbol to the block.
291+
auto &S1 = G.addDefinedSymbol(B1, 0, "S1", 4, Linkage::Strong, Scope::Default,
292+
false, false);
293+
294+
EXPECT_TRUE(S1.isDefined()) << "Symbol should be defined";
295+
EXPECT_FALSE(S1.isExternal()) << "Symbol should not be external";
296+
EXPECT_FALSE(S1.isAbsolute()) << "Symbol should not be absolute";
297+
EXPECT_TRUE(&S1.getBlock()) << "Symbol should have a non-null block";
298+
EXPECT_EQ(S1.getAddress(), orc::ExecutorAddr(0x1000))
299+
<< "Unexpected symbol address";
300+
301+
EXPECT_EQ(
302+
std::distance(G.defined_symbols().begin(), G.defined_symbols().end()), 1U)
303+
<< "Unexpected number of defined symbols";
304+
EXPECT_EQ(
305+
std::distance(G.external_symbols().begin(), G.external_symbols().end()),
306+
0U)
307+
<< "Unexpected number of external symbols";
308+
309+
// Add an external symbol.
310+
auto &S2 = G.addExternalSymbol("S2", 0, Linkage::Strong);
311+
312+
EXPECT_TRUE(S2.isExternal()) << "Symbol should be external";
253313
EXPECT_EQ(
254314
std::distance(G.external_symbols().begin(), G.external_symbols().end()),
255315
1U)
316+
<< "Unexpected number of symbols";
317+
318+
// Make S1 and S2 absolute, confirm that the its flags are updated and that it
319+
// is moved from the defined/external symbols lists to the absolutes list.
320+
orc::ExecutorAddr S1AbsAddr(0xA000);
321+
orc::ExecutorAddr S2AbsAddr(0xB000);
322+
G.makeAbsolute(S1, S1AbsAddr);
323+
G.makeAbsolute(S2, S2AbsAddr);
324+
325+
EXPECT_FALSE(S1.isDefined()) << "Symbol should not be defined";
326+
EXPECT_FALSE(S1.isExternal()) << "Symbol should not be external";
327+
EXPECT_TRUE(S1.isAbsolute()) << "Symbol should be absolute";
328+
EXPECT_FALSE(S2.isDefined()) << "Symbol should not be defined";
329+
EXPECT_FALSE(S2.isExternal()) << "Symbol should not be absolute";
330+
EXPECT_TRUE(S2.isAbsolute()) << "Symbol should be absolute";
331+
332+
EXPECT_EQ(S1.getAddress(), S1AbsAddr) << "Unexpected symbol address";
333+
EXPECT_EQ(S2.getAddress(), S2AbsAddr) << "Unexpected symbol address";
334+
335+
EXPECT_EQ(
336+
std::distance(G.defined_symbols().begin(), G.defined_symbols().end()), 0U)
337+
<< "Unexpected number of defined symbols";
338+
EXPECT_EQ(
339+
std::distance(G.external_symbols().begin(), G.external_symbols().end()),
340+
0U)
341+
<< "Unexpected number of external symbols";
342+
EXPECT_EQ(
343+
std::distance(G.absolute_symbols().begin(), G.absolute_symbols().end()),
344+
2U)
256345
<< "Unexpected number of external symbols";
257346
}
258347

0 commit comments

Comments
 (0)