paulk-asert commented on PR #15557:
URL: https://github.com/apache/grails-core/pull/15557#issuecomment-4190212314

   Claude's analysis from the Groovy side:
   
   ⏺ I now have a deep understanding of the mechanism. Let me provide my 
comprehensive analysis.
                                                                                
                                                                                
                                                                                
                   
     ---                                                                        
                                                                                
                                                                                
                   
     Assessment: The grails-geb Frame.putAbstractType AssertionError
                                                                                
                                                                                
                                                                                
                   
     Definitive Finding: This is a Groovy bug, NOT an ASM bug  
                                                                                
                                                                                
                                                                                
                   
     ASM 9.9 and 9.9.1 are functionally identical. I verified this by 
extracting and diffing ALL source files across ALL five ASM modules (asm-core, 
asm-tree, asm-analysis, asm-util, asm-commons):                                 
                              
                                                                                
                                                                                
                                                                                
                   
     - Frame.java (where the assertion fires): byte-for-byte identical in 
source AND compiled bytecode                                                    
                                                                                
                         
     - MethodWriter.java, SymbolTable.java, Label.java: all identical
     - The only functional change in 9.9.1 is a bounds check in 
ClassReader.readAttribute() for malformed class files — completely unrelated to 
frame computation                                                               
                                   
                                                                                
                                                                                
                                                                                
                   
     The PR author's claim of bisecting to the ASM bump is likely a 
version-level observation (5.0.3 works, 5.0.4 doesn't) rather than a 
commit-level bisect. The ASM bump commit happened to be among 41 commits 
between those releases.                          
                                                                                
                                                                                
                                                                                
                   
     The Assertion Mechanism                                                    
                                                                                
                                                                                
                   
                                                               
     The assertion at Frame.java:1482 fires specifically when:
     1. An abstract type has array dimensions > 0
     2. Its kind is NOT REFERENCE_KIND (it's an unresolved/invalid kind like 
kind=0)                                                                         
                                                                                
                      
     3. Its value doesn't match any known primitive type constant               
    
                                                                                
                                                                                
                                                                                
                   
     This occurs when ASM's getConcreteOutputType() resolves a LOCAL_KIND 
abstract type that references an uninitialized local variable (value=0 in 
inputLocals). The arithmetic dim + inputLocals[localIndex] produces ARRAY_OF + 
0 = 0x04000000, which has dim=1 
     but kind=0 (invalid) and value=0 (ITEM_TOP) — not matching any valid type. 
This gets stored in the destination frame and triggers the assertion during 
serialization.                                                                  
                       
                                                                                
                                                                                
                                                                                
                   
     In plain English: Groovy generates bytecode where a local variable is 
treated as an array on some code path, but that local is uninitialized (never 
assigned a type) on the path that reaches the merge point.                      
                          
                                                               
     Most Likely Culprit Commits (5.0.3 → 5.0.4)                                
                                                                                
                                                                                
                   
                                                               
     The commits that changed bytecode generation between these versions:       
                                                                                
                                                                                
                   
      
     1. GROOVY-11840 (dbd2304527) — Primary suspect                             
                                                                                
                                                                                
                   
       - Rewrote 
BooleanExpressionTransformer.OptimizingBooleanExpression.visit() — how 
non-boolean values are compiled in boolean contexts under @CompileStatic
       - Changed from pre-computed type (TypeChooser.resolveType()) to runtime 
stack type (os.getTopOperand())                                                 
                                                                                
                    
       - Also changed StaticInvocationWriter.makeGetField() — how field access 
receiver types are resolved for ClassExpression receivers                       
                                                                                
                    
       - Also added makeCachedCall() override returning false                   
                                                                                
                                                                                
                   
     2. GROOVY-11817 (920970323e) — Secondary suspect                           
                                                                                
                                                                                
                   
       - Moved DYNAMIC_RESOLUTION check from method-level to expression-level 
for CallSiteWriter selection                                                    
                                                                                
                     
       - This changes which call site writer handles property accesses in trait 
methods — static vs dynamic dispatch could affect local variable typing         
                                                                                
                   
     3. GROOVY-11828 (644796c7bc) — Lower probability                           
                                                                                
                                                                                
                   
       - Changed instanceof variable handling for closure-shared variables      
                                                                                
                                                                                
                   
       - Added SWAP + INVOKEVIRTUAL bytecode sequence for holder variables      
                                                                                
                                                                                
                   
                                                                                
                                                                                
                                                                                
                   
     Why the grails-geb Code Triggers It                                        
                                                                                
                                                                                
                   
                                                                                
                                                                                
                                                                                
                   
     The grails-geb testFixtures have:                                          
                                                                                
                                                                                
                   
     - 6 @CompileStatic traits (BrowserDelegate, PageDelegate, DriverDelegate, 
ContainerSupport, ReportingSupport, DownloadSupportDelegate) with 170+ 
delegating methods
     - ContainerGebSpec implementing all 6 traits with @CompileStatic           
                                                                                
                                                                                
                   
     - GrailsContainerGebExtension with @TailRecursive + @CompileStatic + 
object-in-boolean-context (if (spec) where spec is SpecInfo)
     - WebDriverContainerHolder with complex control flow, boolean expressions 
with non-boolean objects (if (customBrowser), if (gebConf.driverConf))          
                                                                                
                    
     - Many methods with Object[] parameters and generic types (<T extends 
Page>)                                                                          
                                                                                
                        
                                                                                
                                                                                
                                                                                
                   
     The combination of trait helper class generation, static compilation, and 
the BooleanExpressionTransformer rewrite likely produces bytecode where a local 
variable used for an array parameter or return value is uninitialized on a 
particular code path.    
                                                                                
                                                                                
                                                                                
                   
     Recommendation                                                             
                                                                                
                                                                                
                   
                                                                                
                                                                                
                                                                                
                   
     To confirm the exact culprit commit, I'd suggest a git bisect on the 
Groovy 5_0_X branch between GROOVY_5_0_3 and GROOVY_5_0_4, testing each 
intermediate build against the original (unmodified) grails-geb testFixtures.   
                                 
      
     A quicker test: revert just GROOVY-11840 on the 5.0.4 tag and rebuild 
Groovy to see if the assertion disappears. If not, try GROOVY-11817.            
                                                                                
                        
                                                               
     Also, the catch block at AsmClassGenerator.java:624 catches Exception but 
AssertionError extends Error, so the helpful class/method context message is 
never shown. Changing to catch (Throwable e) would immediately tell you which 
class and method triggers
      the failure — worth doing as a diagnostic patch on Groovy.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to