Antlr4: [Swift] Swift-Runtime error after XCode 10 / iOS 12 update

[stonebug-com]

After an update to XCode 10 / iOS12 I get an error in the antlr4 runtime (Swift) when parsing an expression. The reason for this is, that antlr4 stores _ATNConfig_ instances in _sets_ (_closureBusy.insert(c)_ in _ParserATNSimulator_) and modifies the _reachesIntoOuterContext_ attribute of the stored instance afterwards. This can (and does) lead to duplicates in the _set_ since this attribute is significant for the '==' operator of _ATNConfig_ (by _isPrecedenceFilterSuppressed()_), which is used to check if an element is already in the _set_ when it has the same hascode as another element of the _set_ (the _reachesIntoOuterContext_ is not significant for the hashcode).

It seams to me that this error was undiscovered by know and came up now, as the new XCode / iOS release now checks _sets_ for duplicates - what it did not before.

After removing the _isPrecedenceFilterSuppressed()_ comparison from the '==' operator of _ATNConfig_ everything seems to work fine (all my uint tests for my parser work fine again). Since I did not go to much into the deep, I do not know if my modification does have any side effects.

Can you please check if you can confirm the bug and check if my fix is ok?

Cheers
Claude

[ajithrnayak]

Hi there!
So, I'm facing the same crash when module is compiled with Swift 4.2. Currently, We are forced to upload AppStore builds using Xcode 9 since it works just fine.
Swift doesn't has ABI compatibility yet so building only the Antlr4 framework using Xcode 9 and importing it into our project on Xcode 10 doesn't work.

I couldn't figure out the reason for crash myself. I did go through Swift 4.2 Release Notes to find out updates on inout parameters. I could try what @stonebug-com is suggesting.

Fatal error: Duplicate element found in Set. Elements may have been mutated after insertion
error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
The process has been returned to the state before expression evaluation.
Fatal error: Duplicate element found in Set. Elements may have been mutated after insertion

Happy to provide more information about the crash. This is a critical one for us!

[kyouko-taiga]

I face the same issue. If that might be of any help, here's a minimal grammar that can produce the mentioned runtime error:

expression             : number_literal
                       | expression mul_operator expression
                       | expression add_operator expression
                       ;

mul_operator           : '*' ;
add_operator           : '+' ;

number_literal         : '0' | NONZERO_LITERAL ;
NONZERO_LITERAL        : NONZERO_DIGIT DIGIT* ;
fragment NONZERO_DIGIT : [1-9] ;
fragment DIGIT         : [0-9] ;

WHITESPACE             : [ \t\r\n]+ -> skip ;

The solution proposed by @stonebug-com also works for me, but I'd be equally keen to get a confirmation that it's a safe patch.

[parrt]

Howdy. What's the fix for this? Maybe @hanjoes or @ewanmellor As an idea. I can't build and test antlr as of now :(

[ewanmellor]

I'm looking at it now. @stonebug-com or @kyouko-taiga if you have a commit already then please make a pull request, that will save me some time.

[hanjoes]

checking in
@ewanmellor please let me know if there is anything i can help

[ewanmellor]

@parrt @hanjoes This bug looks like it exists in the Java runtime too. Could you check the logic here?

In antlr4/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfig.java, we have:

1. setPrecedenceFilterSuppressed, which can modify this.reachesIntoOuterContext.
2. isPrecedenceFilterSuppressed, which depends on this.reachesIntoOuterContext.
3. The override of equals, which depends on this.isPrecedenceFilterSuppressed.

These three together mean that the definition of equality can change over the lifetime of the ATNConfig instance, which obviously means that adding these to a set, e.g. ATNConfigSet is going to be problematic.

The code in the Swift runtime is all copied directly from the Java runtime, and it seems to me that this logic bug affects both.

The issue with the Swift runtime is that the Swift standard library is now asserting if we end up with duplicates in the set over time (i.e. an instance has been modified while it is a member of the set). I presume with the Java runtime we simply get away with it (as we used to with the Swift runtime).

I don't understand the intended semantics of ATNConfig / ATNConfigSet, so suggestions are welcome. The fix proposed above is to remove the this.isPrecedenceFilterSuppressed check from ATNConfig.equals, but I don't know what the semantics are supposed to be here.

[parrt]

I'm including @sharwell here as I think it is in his code. @ewanmellor I just can't remember what isPrecedenceFilterSuppressed does!!! ugh. #oldman

[parrt]

Sam said that he would look at it in a couple of days. I'm trying to get stringtemplate 4.1 out the door so I will do that first and then hopefully we can get and antlr release ready within a month or so.

[sharwell]

This was fixed for the Java release in #1955. Other targets are expected to update accordingly.

[parrt]

@ewanmellor back to you! :)

[ewanmellor]

I have filed PR #2407. This includes:

1. The port of #1955 mentioned above, so that we no longer change object hashes after the object is in a dictionary.
2. Fixing the other Swift 4.2 deprecation warnings.
3. Fixing a nasty bug with the way that Hashable has changed in 4.2.
4. Moving Travis to Swift 4.2 / Xcode 10.1.

This should be sufficient for Swift 4.2 as far as I can tell. @stonebug-com @ajithrnayak @kyouko-taiga please try this in your environments, and file new bugs if you find any.

[kyouko-taiga]

Works like a charm, thanks!