[compiler-rt] r266295 - [sanitizer] [SystemZ] Implement internal_mmap.

[Marcin Kościelnicki via llvm-commits]

On 15/04/16 20:56, Kostya Serebryany wrote:
> Making such changes is not easy, but please try to find ways to minimize
> the number of ifdefs.
> The more stuff you move to separate SystemZ-specific file(s) -- the better.

Fair enough, I'll split off internal_mmap + internal_clone + the CVE 
2016-2143 business to a separate file.  However, you'll now have a 
#ifndef __s390__ around the "normal" internal_mmap.

What should be done about the existing arch-specific functions?  Should 
I break off all the internal_clone implementations to separate files?
>
> On Fri, Apr 15, 2016 at 10:20 AM, Marcin Kościelnicki <koriakin at 0x04.net
> <mailto:koriakin at 0x04.net>> wrote:
>
>     On 15/04/16 18:48, Kostya Serebryany wrote:
>
>         I'd prefer if you could move this and similar pieces of code into
>         separate SystemZ-specific file(s).
>         The sanitizer run-time is already a hairy ball of #ifdefs, let's
>         try not
>         to make it worse.
>
>
>     Very well, but what exactly are you suggesting instead?
>
>     s390 is just another architecture.  All the others already have lots
>     of ifdefs in the code, I'm merely following the current style:
>
>     projects/compiler-rt/lib $ grep -R __x86_64 *san* | wc -l
>     79
>     projects/compiler-rt/lib $ grep -R __powerpc *san* | wc -l
>     69
>     projects/compiler-rt/lib $ grep -R __aarch *san* | wc -l
>     70
>     projects/compiler-rt/lib $ grep -R __s390 *san* | wc -l
>     60
>     projects/compiler-rt/lib $ grep -R __i386 *san* | wc -l
>     34
>     projects/compiler-rt/lib $ grep -R __arm *san* | wc -l
>     29
>     projects/compiler-rt/lib $ grep -R __mips *san* | wc -l
>     80
>
>     This is with quite a few unsubmitted s390 patches on top of what's
>     already in review or svn.  s390 is certainly on the heavier side,
>     but not out of the ordinary.
>
>     If you want to have arch-specific files for arch-specific functions,
>     fine with me.  But that's not how things are currently done, and
>     you'll still have a maze of #ifdefs left over - there's no sane way
>     to expunge them from eg. D18888.  For things like D18895, you could
>     split it off, but only at the cost of duplicating code.  You'll also
>     still have #ifdefs for disabling the common versions of functions
>     overriden by arch-specific ones (eg. internal_mmap will need #ifndef
>     __s390__).
>
>