Skip to content

Commit 5ed1f93

Browse files
committed
Fix bug: the Slowpath doesn't work correctly because of double increment
builtin_lookup() find the pre-loaded and compiled (to ISeq) builtin libraries. The libraries (.rb files) are loaded by miniruby, and those ISeq are written in build/builtin_library.rbbin in the order that miniruby loads. The fastpath of builtin_lookup() searches the contents of builtin_library.rbbin in the order of miniruby loading, on ruby. The only fastpath works as far as the order of loading libraries in ruby is equal to miniruby. This is the reason why the slowpath's bug has not caused problems. The slowpath incremented `bb` twice in a loop, so it's clearly bug to skip the item after checking bb. Items at 1, 3, 5... will never be found in the swlopath.
1 parent 56cd26f commit 5ed1f93

1 file changed

Lines changed: 28 additions & 5 deletions

File tree

builtin.c

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,36 @@ bin4feature(const struct builtin_binary *bb, const char *feature, size_t *psize)
2222
static const unsigned char*
2323
builtin_lookup(const char *feature, size_t *psize)
2424
{
25-
static int index = 0;
26-
const unsigned char *bin = bin4feature(&builtin_binary[index++], feature, psize);
25+
static size_t index = 0;
26+
const unsigned char *bin = NULL;
27+
28+
/*
29+
* Fast path:
30+
* builtin_binary is usually arranged in the same order
31+
* as features are looked up in miniruby, so try the next entry first.
32+
*/
33+
if (builtin_binary[index].feature) {
34+
bin = bin4feature(&builtin_binary[index], feature, psize);
35+
index++;
36+
}
37+
if (bin) {
38+
return bin;
39+
}
2740

28-
// usually, `builtin_binary` order is loading order at miniruby.
29-
for (const struct builtin_binary *bb = &builtin_binary[0]; bb->feature &&! bin; bb++) {
30-
bin = bin4feature(bb++, feature, psize);
41+
/*
42+
* Fallback:
43+
* In case the lookup order does not match the array order,
44+
* scan the entire table to find the feature.
45+
*/
46+
for (const struct builtin_binary *bb = &builtin_binary[0];
47+
bb->feature;
48+
bb++) {
49+
bin = bin4feature(bb, feature, psize);
50+
if (bin) {
51+
break;
52+
}
3153
}
54+
3255
return bin;
3356
}
3457

0 commit comments

Comments
 (0)