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

[Kostya Serebryany via llvm-commits]

On Fri, Apr 15, 2016 at 11:59 AM, Marcin Kościelnicki <koriakin at 0x04.net>
wrote:

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

Well, it's hard to tell exactly what would be better. (they problem isn't
trivial)
Try something to miniimze the number of #ifdefs and let us look at it.



>
>> 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__).
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160415/e238dd26/attachment.html>