Page MenuHomeWildfire Games

Analysis of unique_range assertion failure on MacOS
Needs ReviewPublic

Authored by Teiresias on Jan 3 2020, 7:45 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#5165
Summary

The associated patch is not (yet) intended as a contribution to 0AD. It's a trimmed down version of the test case introducted by D1511, which caused a Jenkins test error on macos (but not on VS build).

The idea of the patch is to have the little bit of code which utilizes UniqueRange execute without the "overhead" of ArchiveReader_Zip and other code found in archive_zip.cpp. If the test case of this patch still crashes, it indicates that there is a problem in the allocator code.

Currently, no review required as the code is not yet intended for production. However, should the test indicate an error in the allocator code, it has to be extended into a bugfix with review and everything.

Test Plan
  1. Have the Jenkins pipeline build and run/test a 0AD system on all platforms, using the patch
  2. If the test again fails on MacOS, it means that there is either a general problem in the allocator or it's usage is incorrect (again a problem since that code is already in trunk)
  3. If the test case passes on all platforms, the culprit is apparently outside the allocator itself, either in the zip handling patch or in io::Store. The second being unlikely, for a local test run has shown io::Store does not allocate buffers.

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest-debug.xml::[failed-to-read]
Failed to read test report file /Users/wfg/Jenkins/workspace/macos-differential/cxxtest-debug.xml org.dom4j.DocumentException: Error on line 1 of document : Content is not allowed in prolog. at org.dom4j.io.SAXReader.read(SAXReader.java:462)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
View Full Test Results (1 Failed · 994 Passed)

Event Timeline

Teiresias created this revision.Jan 3 2020, 7:45 PM
Vulcan added a comment.Jan 3 2020, 7:47 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/937/display/redirect

Vulcan added a comment.Jan 3 2020, 7:49 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1455/display/redirect

Vulcan added a comment.Jan 3 2020, 7:50 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/33/display/redirect

I've reproduced that assertion locally on macOS 10.14.6 in debug-mode. I've figured out the reason: it seems that OSAtomicAdd64 doesn't work as expected in debug mode, maybe address was misaligned.

Stan added a subscriber: Stan.Jan 21 2020, 10:02 AM

I've reproduced that assertion locally on macOS 10.14.6 in debug-mode. I've figured out the reason: it seems that OSAtomicAdd64 doesn't work as expected in debug mode, maybe address was misaligned.

Can it be fixed?

In D2531#108430, @Stan wrote:

I've reproduced that assertion locally on macOS 10.14.6 in debug-mode. I've figured out the reason: it seems that OSAtomicAdd64 doesn't work as expected in debug mode, maybe address was misaligned.

Can it be fixed?

Sure.

In D2531#108430, @Stan wrote:

I've reproduced that assertion locally on macOS 10.14.6 in debug-mode. I've figured out the reason: it seems that OSAtomicAdd64 doesn't work as expected in debug mode, maybe address was misaligned.

Can it be fixed?

Sure.

I'm pretty sure we need to replace those with standard C++11 functions at some point anyways.

Stan added a comment.May 17 2020, 10:06 PM

Can one of you submit a patch, so @Teiresias can finish his patch eventually? :D

See D613 for some code that removes UniqueRange altogether.