On Wed, Nov 21, 2012 at 6:20 AM, David Miller <da...@davemloft.net> wrote:
> From: David Miller <da...@davemloft.net>
> Date: Tue, 20 Nov 2012 14:59:10 -0500 (EST)
>
>> From: Konstantin Serebryany <konstantin.s.serebry...@gmail.com>
>> Date: Tue, 20 Nov 2012 23:52:48 +0400
>>
>>> Please apply whatever minimal patch required to unbreak the SPARC
>>> build. We will not be accepting any non-trivial patches until we
>>> set up semi-automated way to pull the upstream sources.
>>
>> Will do.
>
> I tossed together a quick sparc implementation and there seems to
> be only two issues to fix:
>
> 1) As noticed by the powerpc people, you have to probe the page
> size of the system properly. It's variable even within a
> target/OS.
>
> Probably a new hook, implemented in asan_linux.cc via:
>
> #include <unistd.h>
>
> uptr GetPageSize()
> {
> return getpagesize();
> }
Today, kPageSize is used in several places where it is expected to be
a compile-time constant.
Even if it seems like replacing it with GetPageSize() is safe, it
would need very significant testing (including performance testing).
Can we just define kPageSize=1UL<<13 under ifdef __sparc__?
What are the possible page size values for SPARC?
>
> I would just get rid of kPageSizeBits,
Indeed, this is not used any more. Removed.
http://llvm.org/viewvc/llvm-project?rev=168426&view=rev
--kcc
> rather than compute it
> dynamically as well, as it's really not used as far as I can tell.
>
> The mmap of the shadow area won't work on sparc without this being
> resolved.
>
> 2) The current code emitted to set the shadow values results in
> unaligned stores. For example, for the memcmp-1.c test on 32-bit
> we get:
>
> 0x10488 <main+8>: add %fp, -160, %i5
> ...
> 0x10490 <main+16>: sethi %hi(0xf1f1f000), %g2
> ...
> 0x104a0 <main+32>: srl %i5, 3, %i5
> ...
> 0x104a8 <main+40>: or %g2, 0x1f1, %g2
> 0x104ac <main+44>: sethi %hi(0x20000000), %g1
> ...
> => 0x104b4 <main+52>: st %g2, [ %i5 + %g1 ]
>
> Here %fp is 0xffffd538, and this %i5 + %g1 is 0x3ffffa93, which
> is not aligned properly for a 32-bit store.
>
> Therefore, this won't work for any STRICT_ALIGNMENT target.
>
> Those seem to be the only problems that need to be resolved for this
> feature to be fully working.
