Re: An aliasing problem

[Bruno H. <br...@cl...>]

Hi Jerry,

You wrote in <https://sourceforge.net/p/clisp/mailman/message/35687181/>:
> *** - Internal error: statement in file "../src/record.d", line 1543 has been
>       reached!!
>       Real time: 29.498787 sec.
> ...
> 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.

Wow! You hit the nail on the head.

> 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.

The clisp code has many situations like this. It's not only between
TheRecord and TheInstance, but surely also between
TheRecord and ThePackage, TheRecord and TheHashtable,
TheVarobject and TheSstring, and so on. Therefore the result of the
solution you are hinting at would be a huge union type with 50 alternatives.
At a lot of clisp code to rewrite.

I'm choosing the follow way you went: add a compiler option.

Basically, I want a compiler that translates that statements I wrote,
not a compiler that makes optimizations on the premise that the code
I wrote is buggy when it violates the letter of C99.

Bruno

--- a/src/makemake.in   Sun Feb 26 18:27:03 2017 +0100
+++ b/src/makemake.in   Sun Feb 26 18:30:41 2017 +0100
@@ -1325,6 +1327,23 @@
 
 fi
 
+# We access the same memory through pointers of different types, for example
+# as TheVarobject(obj)->..., TheRecord(obj)->..., TheInstance(obj)->... .
+# This violates the strict type-based aliasing rules of C. In other words, we
+# still use C as a portable assembler, but now the compilers want to outsmart
+# us. There are two ways to tell them not to do this: to use union types, or
+# specific compiler options. I prefer to do it through compiler options.
+if [ $XCC_GCC = true ] ; then
+  XCFLAGS=$XCFLAGS' -fno-strict-aliasing'
+else
+  if [ "$HSYSOS" = aix ] ; then # for xlc
+    XCFLAGS=$XCFLAGS' -qalias=noansi'
+  fi
+  if [ -n "$XCC_SUNPRO" ] ; then # for SUNWspro cc
+    XCFLAGS=$XCFLAGS' -xalias_level=weak'
+  fi
+fi
+
 if [ "${with_dynamic_modules}" != no ]; then # not on msvc
   # Support for dynamic loading.
   eval "`./libtool --tag=CC --config | grep '^pic_flag='`"