Opened 7 years ago

Closed 5 years ago

#1122 closed defect (fixed)

Some compiler rewrites marked "safe" are really unsafe

Reported by: sjamaan Owned by:
Priority: major Milestone: 4.11.0
Component: compiler Version: 4.9.x
Keywords: Cc:
Estimated difficulty:

Description

Due to a currently unknown reason, sometimes the scrutinizer seems to be able to prevent hardwired compiler rewrites from being done.

If the following code is interpreted, it will print two lines saying "error". If it is compiled, it will either print "error" and "smaller", or in case of a DEBUGBUILD, it will print "error" and show a low-level type assertion error.

(use ports)
(handle-exceptions exn
    (print 'error)
    (print (if (char<? 1 #\a) 'smaller 'bigger-or-equal) ))
(handle-exceptions exn
    (print 'error)
    (print (if (char<? (with-input-from-string "1" read) #\a)
               'smaller 'bigger-or-equal)))

Perhaps it's a good idea to remove the compiler rewrites completely (they are in c-platform.scm), and, where suitable, move the rewrites to types.db. This will prevent bogus rewrites like the one above, and should also make the compiler simpler. We must ensure this doesn't mess up performance too bad. Benchmarking will help.

Change History (4)

comment:1 Changed 5 years ago by sjamaan

Milestone: 4.10.04.11.0

See also #1216; it shows that this bug is more serious than previously thought

comment:2 Changed 5 years ago by sjamaan

OK, the "unknown reason" is now known: The topmost call won't get replaced due to attempted constant-folding via constant-form-eval, which fails (due to the exception), and therefore added to the broken-constant-nodes list.

These broken constant nodes are exempt from simplification in optimizer.scm, right at the top of walk in perform-high-level-optimizations. This is unrelated to #1216, I think.

comment:3 Changed 5 years ago by sjamaan

Summary: Strange interaction between scrutinizer and hardwired compiler rewrites: remove compiler rewrites?Some compiler rewrites marked "safe" are really unsafe

comment:4 Changed 5 years ago by sjamaan

Resolution: fixed
Status: newclosed

Fixed with 379a019 / 274e7af

Note: See TracTickets for help on using tickets.