An aliasing problem

[Jerry J. <log...@gm...>]

It's great that development on clisp has picked back up.  Thanks, Sam
and Bruno, for all your work.

Fedora recently rebuilt all packages with GCC 7.  The i386 and x86_64
builds succeeded, but the 32-bit ARM build failed, like so:

(PROGN (DEFMETHOD UPDATE-INSTANCE-FOR-REDEFINED-CLASS :BEFORE ((POS
X-Y-POSITION) ADDED DELETED PLIST &KEY) (LET ((X (GETF PLIST 'X)) (Y
(GETF PLIST 'Y))) (SETF (POSITION-RHO POS) (SQRT (+ (* X X) (* Y Y)))
(POSITION-THETA POS) (ATAN Y X)))) (DEFCLASS X-Y-POSITION
(ABSTRACT-POSITION) ((RHO :INITFORM 0 :ACCESSOR POSITION-RHO) (THETA
:INITFORM 0 :ACCESSOR POSITION-THETA))) (DEFMETHOD POSITION-X ((POS
X-Y-POSITION)) (WITH-SLOTS (RHO THETA) POS (* RHO (COS THETA))))
(DEFMETHOD (SETF POSITION-X) (NEW-X (POS X-Y-POSITION)) (WITH-SLOTS
(RHO THETA) POS (LET ((Y (POSITION-Y POS))) (SETQ RHO (SQRT (+ (*
NEW-X NEW-X) (* Y Y))) THETA (ATAN Y NEW-X)) NEW-X))) (DEFMETHOD
POSITION-Y ((POS X-Y-POSITION)) (WITH-SLOTS (RHO THETA) POS (* RHO
(SIN THETA)))) (DEFMETHOD (SETF POSITION-Y) (NEW-Y (POS X-Y-POSITION))
(WITH-SLOTS (RHO THETA) POS (LET ((X (POSITION-X POS))) (SETQ RHO
(SQRT (+ (* X X) (* NEW-Y NEW-Y))) THETA (ATAN NEW-Y X)) NEW-Y)))
(LIST (TYPE-OF I) (POSITION-X I) (POSITION-Y I) (POSITION-RHO I)
(POSITION-THETA I)))
WARNING: DEFCLASS: Class X-Y-POSITION (or one of its ancestors) is being
         redefined, instances are obsolete
WARNING: Removing method
         #1=#<STANDARD-WRITER-METHOD (#2=#<BUILT-IN-CLASS T>
#3=#<STANDARD-CLASS X-Y-POSITION :VERSION 1>)>
         from an already called generic function
         #<STANDARD-GENERIC-FUNCTION (SETF POSITION-Y)>
WARNING: Removing method
         #1=#<STANDARD-WRITER-METHOD (#2=#<BUILT-IN-CLASS T>
#3=#<STANDARD-CLASS X-Y-POSITION :VERSION 1>)>
         from an already called generic function
         #<STANDARD-GENERIC-FUNCTION (SETF POSITION-X)>
WARNING: DEFCLASS: Class X-Y-POSITION (or one of its ancestors) is being
         redefined, instances are obsolete
*** - Internal error: statement in file "../src/record.d", line 1543 has been
      reached!!
      Real time: 29.498787 sec.
Run time: 28.986 sec.
Space: 35364284 Bytes
GC: 70, GC time: 0.853 sec.
Please see <http://clisp.org/impnotes/faq.html#faq-bugs> for bug
      reporting instructions.
Bye.

After fruitlessly staring at the source code for awhile, trying to
divine how this could possibly happen, I decided to stare at the
preprocessed code instead.  I made it more readable by removing some
extra parentheses, reformatting, replacing some equivalent types, and
calculating some compile-time constants:

    obj = mv_space[0];
    ((Instance)((unsigned long)obj-1UL))->tfl |= 2048UL;
    {
      break_sems.einzeln[1] = 1;
      {
        Instance ptr = (Instance)((unsigned
long)STACK[-1-(long)(2+4+2*kept_slots)]-1UL);
        ptr->tfl |= 256UL;
        ptr->inst_class_version = obj;
        break_sems.einzeln[1] = 0;
      }
    }
    if (!(((((Record)((unsigned
long)STACK[-1-(long)(2+4+2*kept_slots)]-1UL))->tfl >> 8) & 0xFF) &
1UL))
      error_notreached("../src/record.d",1543);

It dawned on me that this might be an aliasing issue.  The pointer
derived from ((unsigned long)STACK[-1-(long)(2+4+2*kept_slots)]-1UL)
is visible in this scope twice, once cast to an Instance (pointer to
an anonymous struct) and once cast to a Record (record_ *).  I tried
building with -fno-strict-aliasing and sure enough, the build
succeeded.

What do you think about changing line 1543 to:

    ASSERT(record_flags(TheInstance(STACK_(2+4+2*kept_slots))) &
instflags_forwarded_B);

, or maybe making an Instance_flags(obj) macro that expands to
record_flags(TheInstance(obj))?  That makes the two pointers have the
same type, thereby solving the aliasing issue.  I have done a test
build with this change and it does indeed fix the problem.  Regards,
-- 
Jerry James
http://www.jamezone.org/