Skip to content

Conversation

@StephanTLavavej
Copy link
Member

While working on internal MSVC-PRs, including mirroring GitHub changes, I noticed something curious. The Standard Library Header Units test appeared to be ICEing, but it wasn't related to any of the PR changes. What happened is that our internal "chk" compiler (with assertions enabled) had begun ICEing in this test. (We weren't seeing it on GitHub because either the shipping "ret" compiler was less severely affected, or because recent changes that intensified the problem hadn't yet shipped. I didn't care enough to investigate which was the case.) The mysterious thing was why this ICE wasn't blocking our internal PR/CI checks. While the ICE had a different underlying cause (internal VSO-2293247 "/Zc:preprocessor does not terminate macro definitions properly", which @cdacamar just fixed with internal MSVC-PR-588923), the specific way the STL was encountering it was through the /scanDependencies compiler option, which is used only by the Standard Library Header Units test, and because that test requires the most complicated custom build steps, we have a Perl script custombuild.pl that drives it. The ICE was causing the Perl script to terminate in a way that the rest of the internal test harness didn't expect, so the test was reported as "skipped" instead of "failed" or "cascaded". That's why our PR/CI checks weren't noticing the ICE, and only if there was another failure would the full test logs contain the record of the ICE (which is how I noticed this - it was a very long story, with the internal perennial test suite noticing that #4958 didn't handle heterogeneous types in adjacent_difference correctly).

I found that we need to use the internal function Run::PrintError instead of Perl die. In the aftermath of the ICE, the compiler's /scanDependencies emitted bogus JSON (I think it was simply empty), so I'm wrapping the JSON decoding in Perl try/catch.

I've verified (before the ICE was fixed) that this now correctly reports test failure, so we'll avoid future regressions of this form.

Click to expand test failure log:
failure: CPPTEST-72-089 std tests\P1502R1_standard_library_header_units 1 1 1 1 1 1
[...]
RUN.PL line 74:  Using custombuild.pl for build step
RUN.PM line 754:  Executing: "cl  /nologo /Od /W4 /w14061 /w14242 /w14265 /w14582 /w14583 /w14587 /w14588 /w14749 /w14841 /w14842 /w15038 /w15214 /w15215 /w15216 /w15217 /w15262 /sdl /WX /D_ENABLE_STL_INTERNAL_CHECK /bigobj /w14365 /D_ENFORCE_FACET_SPECIALIZATIONS=1 /D_STL_CALL_ABORT_INSTEAD_OF_INVALID_PARAMETER /Zc:preprocessor /w14640 /Zc:threadSafeInit- /EHsc /DTEST_STANDARD=20 /std:c++20 /MD /DTEST_HEADER_UNITS /DTEST_TOPO_SORT  /d1db178,179  /exportHeader /headerName:angle /translateInclude /Fo /MP /scanDependencies .\ __msvc_bit_utils.hpp __msvc_chrono.hpp __msvc_cxx_stdatomic.hpp __msvc_filebuf.hpp __msvc_format_ucd_tables.hpp __msvc_formatter.hpp __msvc_heap_algorithms.hpp __msvc_int128.hpp __msvc_iter_core.hpp __msvc_minmax.hpp __msvc_ostream.hpp __msvc_print.hpp __msvc_ranges_to.hpp __msvc_ranges_tuple_formatter.hpp __msvc_sanitizer_annotate_container.hpp __msvc_string_view.hpp __msvc_system_error_abi.hpp __msvc_threads_core.hpp __msvc_tzdb.hpp __msvc_xlocinfo_types.hpp algorithm any array atomic barrier bit bitset cctype cerrno cfenv cfloat charconv chrono cinttypes climits clocale cmath codecvt compare complex concepts condition_variable coroutine csetjmp csignal cstdarg cstddef cstdint cstdio cstdlib cstring ctime cuchar cwchar cwctype deque exception execution expected filesystem format forward_list fstream functional future generator initializer_list iomanip ios iosfwd iostream iso646.h istream iterator latch limits list locale map mdspan memory memory_resource mutex new numbers numeric optional ostream print queue random ranges ratio regex scoped_allocator semaphore set shared_mutex source_location span spanstream sstream stack stacktrace stdexcept stdfloat stop_token streambuf string string_view strstream syncstream system_error thread tuple type_traits typeindex typeinfo unordered_map unordered_set utility valarray variant vector xatomic.h xatomic_wait.h xbit_ops.h xcall_once.h xcharconv.h xcharconv_ryu.h xcharconv_ryu_tables.h xcharconv_tables.h xerrc.h xfacet xfilesystem_abi.h xhash xiosbase xlocale xlocbuf xlocinfo xlocmes xlocmon xlocnum xloctime xmemory xnode_handle.h xpolymorphic_allocator.h xsmf_control.h xstring xthreads.h xtimec.h xtr1common xtree xutility ymath.h version yvals.h yvals_core.h /link /MANIFEST:EMBED".
__msvc_bit_utils.hpp
__msvc_chrono.hpp
[...]
E:\a01\_work\1\s\binaries\x86chk\inc\utility(15): fatal error C1001: Internal compiler error.
(compiler file 'msc1.cpp', line 1533)
 To work around this problem, try simplifying or changing the program near the locations listed above.
If possible please provide a repro here: https://developercommunity.visualstudio.com 
Please choose the Technical Support command on the Visual C++ 
 Help menu, or open the Technical Support help file for more information (compiling source file E:\a01\_work\1\s\binaries\x86chk\inc\barrier)
INTERNAL COMPILER ERROR in 'E:\a01\_work\1\s\binaries\x86chk\bin\i386\cl.exe'
    Please choose the Technical Support command on the Visual C++
    Help menu, or open the Technical Support help file for more information
cl : Command line error D8040 : error creating or communicating with child process
RUN.PM line 757:  Execution of: "cl  /nologo /Od /W4 /w14061 /w14242 /w14265 /w14582 /w14583 /w14587 /w14588 /w14749 /w14841 /w14842 /w15038 /w15214 /w15215 /w15216 /w15217 /w15262 /sdl /WX /D_ENABLE_STL_INTERNAL_CHECK /bigobj /w14365 /D_ENFORCE_FACET_SPECIALIZATIONS=1 /D_STL_CALL_ABORT_INSTEAD_OF_INVALID_PARAMETER /Zc:preprocessor /w14640 /Zc:threadSafeInit- /EHsc /DTEST_STANDARD=20 /std:c++20 /MD /DTEST_HEADER_UNITS /DTEST_TOPO_SORT  /d1db178,179  /exportHeader /headerName:angle /translateInclude /Fo /MP /scanDependencies .\ __msvc_bit_utils.hpp __msvc_chrono.hpp __msvc_cxx_stdatomic.hpp __msvc_filebuf.hpp __msvc_format_ucd_tables.hpp __msvc_formatter.hpp __msvc_heap_algorithms.hpp __msvc_int128.hpp __msvc_iter_core.hpp __msvc_minmax.hpp __msvc_ostream.hpp __msvc_print.hpp __msvc_ranges_to.hpp __msvc_ranges_tuple_formatter.hpp __msvc_sanitizer_annotate_container.hpp __msvc_string_view.hpp __msvc_system_error_abi.hpp __msvc_threads_core.hpp __msvc_tzdb.hpp __msvc_xlocinfo_types.hpp algorithm any array atomic barrier bit bitset cctype cerrno cfenv cfloat charconv chrono cinttypes climits clocale cmath codecvt compare complex concepts condition_variable coroutine csetjmp csignal cstdarg cstddef cstdint cstdio cstdlib cstring ctime cuchar cwchar cwctype deque exception execution expected filesystem format forward_list fstream functional future generator initializer_list iomanip ios iosfwd iostream iso646.h istream iterator latch limits list locale map mdspan memory memory_resource mutex new numbers numeric optional ostream print queue random ranges ratio regex scoped_allocator semaphore set shared_mutex source_location span spanstream sstream stack stacktrace stdexcept stdfloat stop_token streambuf string string_view strstream syncstream system_error thread tuple type_traits typeindex typeinfo unordered_map unordered_set utility valarray variant vector xatomic.h xatomic_wait.h xbit_ops.h xcall_once.h xcharconv.h xcharconv_ryu.h xcharconv_ryu_tables.h xcharconv_tables.h xerrc.h xfacet xfilesystem_abi.h xhash xiosbase xlocale xlocbuf xlocinfo xlocmes xlocmon xlocnum xloctime xmemory xnode_handle.h xpolymorphic_allocator.h xsmf_control.h xstring xthreads.h xtimec.h xtr1common xtree xutility ymath.h version yvals.h yvals_core.h /link /MANIFEST:EMBED" returned "2".
CUSTOMBUILD.PL line 26 - Error:  Caught exception in loadJson.

RUN.PM: Test Cascaded.

I don't believe that our custom_format.py for the GitHub test harness is affected at all - when stuff goes wrong, the Lit machinery is good about not silently ignoring it.

Also, while our Standard Library Modules test has a custombuild.pl of its own, it's far less complicated and doesn't /scanDependencies or decode JSON, so I believe it's unaffected.

@StephanTLavavej StephanTLavavej added test Related to test code modules C++23 modules, C++20 header units labels Oct 30, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner October 30, 2024 18:01
@StephanTLavavej

This comment was marked as resolved.

@CaseyCarter CaseyCarter self-requested a review October 30, 2024 19:20
@CaseyCarter CaseyCarter removed their assignment Oct 30, 2024
@StephanTLavavej
Copy link
Member Author

I've pushed an additional commit to move the return out of the try. Turns out Perl is weird (who knew?) and the Internet indicates that Try::Tiny is so minimal it doesn't handle return within try (since what try is doing is something like spawning a sub-Perl, and dealing with what happens if the sub-script dies/croaks). A variable assignment definitely sidesteps any headaches here. (As we use a decrepit version of Perl internally, the mechanisms available to use are limited, and I barely know what I'm doing in the first place.)

I validated this by damaging an unrelated test to #error, so I could see any of the "skipped" behavior. With this new variable-assignment logic, the ICEing compiler was properly reported as a "failure", same as in my initial PR description. After cherry-picking the compiler fix, the Header Units test passes normally, no skips or failures.

@StephanTLavavej StephanTLavavej self-assigned this Nov 7, 2024
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit f70b885 into microsoft:main Nov 8, 2024
@StephanTLavavej StephanTLavavej deleted the perlae-imperii branch November 8, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modules C++23 modules, C++20 header units test Related to test code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants