Skip to content

Commit 8d50857

Browse files
author
Robin Kruppe
committed
Check *all* errors in LLVMRustArchiveIterator* API
Incrementing the `Archive::child_iterator` fetches and validates the next child. This can trigger an error, which we previously checked on the *next* call to `LLVMRustArchiveIteratorNext()`. This means we ignore the last error if we stop iterating halfway through. This is harmless (we don't access the child, after all) but LLVM 4.0 calls `abort()` if *any* error goes unchecked, even a success value. This means that basically any rustc invocation that opens an archive and searches through it would die. The solution implemented here is to change the order of operations, such that advancing the iterator and fetching the newly-validated iterator happens in the same `Next()` call. This keeps the error handling behavior as before but ensures all `Error`s get checked.
1 parent c74ac6c commit 8d50857

File tree

1 file changed

+26
-9
lines changed

1 file changed

+26
-9
lines changed

src/rustllvm/ArchiveWrapper.cpp

+26-9
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,15 @@ struct RustArchiveMember {
3333

3434

3535
struct RustArchiveIterator {
36+
bool first;
3637
Archive::child_iterator cur;
3738
Archive::child_iterator end;
3839
#if LLVM_VERSION_GE(3, 9)
3940
Error err;
4041

41-
RustArchiveIterator() : err(Error::success()) { }
42+
RustArchiveIterator() : first(true), err(Error::success()) { }
43+
#else
44+
RustArchiveIterator() : first(true) { }
4245
#endif
4346
};
4447

@@ -120,6 +123,7 @@ LLVMRustArchiveIteratorNew(LLVMRustArchiveRef ra) {
120123
rai->cur = ar->child_begin(rai->err);
121124
if (rai->err) {
122125
LLVMRustSetLastError(toString(std::move(rai->err)).c_str());
126+
delete rai;
123127
return NULL;
124128
}
125129
#endif
@@ -129,27 +133,40 @@ LLVMRustArchiveIteratorNew(LLVMRustArchiveRef ra) {
129133

130134
extern "C" LLVMRustArchiveChildConstRef
131135
LLVMRustArchiveIteratorNext(LLVMRustArchiveIteratorRef rai) {
136+
if (rai->cur == rai->end) return nullptr;
137+
138+
// Advancing the iterator validates the next child, and this can
139+
// uncover an error. LLVM requires that we check all Errors,
140+
// so we only advance the iterator if we actually need to fetch
141+
// the next child.
142+
// This means we must not advance the iterator in the *first* call,
143+
// but instead advance it *before* fetching the child in all later calls.
144+
if (!rai->first) {
145+
++rai->cur;
132146
#if LLVM_VERSION_GE(3, 9)
133-
if (rai->err) {
134-
LLVMRustSetLastError(toString(std::move(rai->err)).c_str());
135-
return NULL;
136-
}
147+
if (rai->err) {
148+
LLVMRustSetLastError(toString(std::move(rai->err)).c_str());
149+
return nullptr;
150+
}
137151
#endif
138-
if (rai->cur == rai->end)
139-
return NULL;
152+
} else {
153+
rai->first = false;
154+
}
155+
156+
if (rai->cur == rai->end) return nullptr;
157+
140158
#if LLVM_VERSION_EQ(3, 8)
141159
const ErrorOr<Archive::Child>* cur = rai->cur.operator->();
142160
if (!*cur) {
143161
LLVMRustSetLastError(cur->getError().message().c_str());
144-
return NULL;
162+
return nullptr;
145163
}
146164
const Archive::Child &child = cur->get();
147165
#else
148166
const Archive::Child &child = *rai->cur.operator->();
149167
#endif
150168
Archive::Child *ret = new Archive::Child(child);
151169

152-
++rai->cur;
153170
return ret;
154171
}
155172

0 commit comments

Comments
 (0)