Re: CVS commit: pkgsrc

[Chuck Cranor <chuck%ece.cmu.edu@localhost>]

On Thu, Jun 09, 2022 at 10:40:37AM +0200, Joerg Sonnenberger wrote:
> On Wed, Jun 08, 2022 at 07:00:13PM +0000, Chuck Cranor wrote:
> > [1] compiler bug in some versions of clang when compiling with "-O2" ...
> >     this appears to have been fixed somewhere between clang-11 and clang-12.
> >     it works ok with "-O1" and it doesn't impact my NetBSD system (which
> >     has gcc instead of clang).   resolve by using memcpy() in LONGCOPY()
> >     macro rather than using custom code that triggers the clang issue.  
> >     see: https://mail-index.netbsd.org/tech-pkg/2022/06/08/msg026367.html
> 
> This code smells like a violation of aliasing rules. Is LONGCOPY ever
> used with overlapping memory? Because in that case the change is not 
> correct.
  
I don't think LONGCOPY is ever used with overlapping memory.  looking at
the code I see that LONGCOPY is only used in 4 places, all in the 
t1lib-5.1.2/lib/type1 directory:
  
   
   1. object.c in t1_Allocate(): 
         LONGCOPY(r, template, size)
             dst buf "r" was just malloc'd by t1_Allocate()        
             src buf "template" is an arg to t1_Allocate() 
                
             since "r" was just malloc'd there can't be overlap with template
 
   2. regions.c in NewEdge():
         LONGCOPY(&r[1], xvalues, ...)
            dst buf "r" was just malloc'd (with extra space) and returned
                    from a call to t1_Allocate()   
            src buf "xvalues" is an arg to NewEdge()
             
            since "r" was just malloc'd by t1_Allocate() there can't be 
            overlap with xvalues   
 
   3. spaces.c in FindContext(M):
         LONGCOPY(contexts[i].normal, M, sizeof(contexts[i].normal))
            dst buf "contexts[i].normal" is a static global variable
            src buf "M" is an arg to FindContext().

            the only function that calls FindContext() is FindDeviceContext()
            and that allocates "M" on its stack as a local variable, so it
            cannot overlap with the static global contexts[i].normal.
          
   4. spaces.c in Context():
          LONGCOPY(M, contexts[n].normal, sizeof(M))
             dst buf "M" one of Context()'s local variables (on stack)
             src buf "contexts[n].normal" is a static global variable

             since "M" is a local var and "contexts[]" is a global var,
             they will not overlap.


The justification for having LONGCOPY() is this comment in object.c:

/* 
:h2.TYPE1IMAGER Object Functions
   
:h3.LONGCOPY() - Macro to Copy "long" Aligned Data
  
Copying arbitrary bytes in C is a bit of a problem.  "strcpy" can't be
used, because 0 bytes are special-cased.  Most environments have a
routine "memcopy" or "bcopy" or "bytecopy" that copies memory containing
zero bytes.  Sadly, there is no standard on the name of such a routine,
which makes it impossible to write truely portable code to use it.

It turns out that TYPE1IMAGER, when it wants to copy data, frequently
knows that both the source and destination are aligned on "long"
boundaries.  This allows us to copy by using "long *" pointers.  This
is usually very efficient on almost all processors.  Frequently, it
is more efficient than using general-purpose assembly language routines.
So, we define a macro to do this in a portable way.  "dest" and "source"
must be long-aligned, and "bytes" must be a multiple of "sizeof(long)":
*/


in the test program i posted that triggers the clang code generation
issue, there is no buffer overlap either.

chuck