boost::filesystem::directory_iterator(const path& p) iteration to cause a std::terminate()

[florent.castelli@…]

Under some conditions, an iteration (for example using a C++11 for loop) over a directory_iterator can lead to a std::terminate()

Happens with Clang from Xcode 7.3.1 on OSX in Release mode (-O2).

Apparently, boost::filesystem::directory_iterator::increment is NOEXCEPT, but it is forwarding the "nullptr" error_code to the underlying function directory_iterator_increment. This one will throw on error if the error_code is "nullptr".

The compiler optimization inlined the call into boost::filesystem::directory_iterator::increment and sees it's NOEXCEPT and thus calls std::terminate() directly and no try / catch in the caller will be able to intercept the errors.

My program was fixed using the second constructor that takes an error_code, but I don't think the following code should hard crash programs:

Local code (sorry, no repro available, the bug should be obvious without it):

try {
  for (auto &directory_entry : boost::filesystem::directory_iterator(path)) {
    // Do things
  }
} catch (const boost::filesystem::filesystem_error &) {
  // swallow all errors but never reached
}

Stacktrace:

* thread #2: tid = 0xe89aef, 0x00007fff9ac3cf06 libsystem_kernel.dylib`__pthread_kill + 10, name = 'Thread name', stop reason = signal SIGABRT
  * frame #0: 0x00007fff9ac3cf06 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff921b24ec libsystem_pthread.dylib`pthread_kill + 90
    frame #2: 0x00007fff8da386e7 libsystem_c.dylib`abort + 129
    frame #3: 0x00007fff91880f81 libc++abi.dylib`abort_message + 257
    frame #4: 0x00007fff918a6a2f libc++abi.dylib`default_terminate_handler() + 243
    frame #5: 0x00007fff96fbb6c3 libobjc.A.dylib`_objc_terminate() + 124
    frame #6: 0x00007fff918a419e libc++abi.dylib`std::__terminate(void (*)()) + 8
    frame #7: 0x00007fff918a4213 libc++abi.dylib`std::terminate() + 51
    frame #8: 0x00000001000034ab storage_test`__clang_call_terminate + 11
    frame #9: 0x0000000100125be0 storage_test`boost::filesystem::directory_iterator::increment(this=<unavailable>, ec=<unavailable>) + 32 at operations.hpp:913
    frame #10: 0x0000000100125125 storage_test`boost::filesystem::detail::directory_iterator_construct(it=0x0000700000080c68, p=<unavailable>, ec=0x0000000000000000) + 453 at operations.cpp:2286
    frame #11: 0x000000010012653d storage_test`boost::filesystem::directory_iterator::directory_iterator(this=<unavailable>, p=<unavailable>) + 61 at operations.hpp:903
    frame #12: 0x0000000100126499 storage_test`boost::filesystem::directory_iterator::directory_iterator(this=<unavailable>, p=<unavailable>) + 9 at operations.hpp:903
    frame #13: 0x000000010009829c storage_test`doStuff(this=0x0000000102805000, base_path="/var/folders/cc/v8qm1vps6fbfd8nnn761ys8w0000gn/T//8a4b3e25b7e08c81385601451ad4fae8d18948bb/storage_test/", files=0x0000700000080de0 size=0, allow_sleep=<unavailable>, where=1) + 236 at storage_impl.cpp:1434

[ajay_sonawane@…]

Do you have any updates on this ? Is there any workaround for this problem?

[anonymous]

Do you have any updates on this ? Is there any workaround for this problem?

[florent.castelli@…]

Workaround is to use the equivalent function returning an error code instead of throwing an exception. Haven't tried the code with 1.63 since I don't have access to the code base anymore.

[Matthew DeLoera <mmdeloera@…>]

I'm dealing with this issue in 1.64. The description isn't accurate, and there are actually multiple related issues.

Issue 1

• I'm working with the Win32 build BTW, but I expect all platforms are broken.
• Referring to include/boost/filesystem/operations.hpp and libs/filesystem/src/operations.cpp.
• Both the except/noexcept constructors of directory_iterator (operations.hpp lines 901-905) call the same detail::directory_iterator_construct, either passing 0 or &ec as normal convention. This is fine.
• Win32's FindFileFirst typically first returns "." for me, so directory_iterator_construct internally calls it.increment(*ec). This is NOT fine - you have just obfuscated the pointer with a reference.
• So directory_iterator::increment always gets a system::error_code &ec, but it's a bad reference if I had used the except constructor. This is the piece that needs changed. Pass error_code*, not error_code&.
• So even though obviously bad reference, non-NULL pointer is passed to detail::directory_iterator_increment. If dir_itr_increment/FindNextFile fails (and sets temp_ec), ec will always be non-NULL, so it will never throw, but the ec->assign call will immediately burst into flames (i.e. access violation). This is the actual crash, not an unexpected exception as suggested above.
• FYI, I encountered this issue using URIs over SMB, rather than local file system. I occasionally get successful FindFirstFile but failed FindNextFile, but that's a tangent here.

Issue 2

• As the original description does mention, please remove BOOST_NOEXCEPT from directory_iterator::increment, since it's obviously not true. Er that is, it's only true right now because your throw is always skipped. But once you fix that, you'll want to do this too.

Issue 3

• When you branch on ec==0 to either throw or not throw within directory_iterator_increment (operations.cpp line 2401), you're always obliterating the error code. Hence, when using non-exception directory_iterator, it's impossible to distinguish between a valid empty directory or internal iteration errors.
• Fix Issue 1.
• Now, instead throw or return temp_ec. Do NOT call BOOST_ERRNO! If dir_itr_increment/FindNextFile failed, it grabs Win32 GetLastError into temp_ec as it should, but then cleans up with Win32's FindClose, which obviously clears the static errno. This 2nd call to BOOST_ERRNO is pretty much guaranteed to return 0.

Issue 4

• In general operations.cpp calls GetLastError way more than it needs/should. I suspect there are other places where a subsequent GetLastError gets obliterated because of prior resource cleanup. I'd suggest grep and confirm. This isn't quite so urgent, but please do yourself a favor.

BTW, I'm willing to pitch in with these fixes if that might be helpful.

[Matthew DeLoera <mmdeloera@…>]

Hence, there actually is no workaround, at least in the case of swallowed errors (Issue 3).

[Matthew DeLoera <mmdeloera@…>]

Actually, is this really the same bug? I'm having trouble telling, but I'm also not using clang.

[Andrey Semashev]

I believe, the problems outlined in the description and comment 3 are fixed as of ​https://github.com/boostorg/filesystem/commit/8c9bba511cce978c128b22c8d2fbfa5ab9ba9d99.