Linux/ARM: Bus error with -O3 flag of clang/llvm-3.6 while running unit-test of .NET Core

Mon Jul 18 19:20:04 PDT 2016

I would like to thank Tim Northover and Renato Golin for their valuable
feedback on this issue.
Below  is the execution result of the 9873 unit tests after generating
CoreCLR with -O3 flag.
I hope that clang/llvm developers can fix easily a similar case via this
emailing list.
( For example, jackd1: bus error on beagleboard -
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=692562 )

However, I am not sure why we can not resolve this issue on LLVM back-end
without CoreCLR level.

* Before applying the patch:
=======================
     Test Results (Release Build + O3)
=======================
# CoreCLR Bin Dir  :
/unit-test/bin.coreclr.release.20160713.1.O3/Product/Linux.arm.Release
# Tests Discovered : 9873
# Passed           : 5626
# Failed           : 3886 (commit: 20160627-52570683, OOM: 1, manual kill
commands: 7 ) <==== HERE!!!
# Skipped          : 361
=======================
102 minutes and 10 seconds taken to run CoreCLR tests.

real 103m44.120s
user 256m19.410s
sys 13m29.340s
root at arm-target:/unit-test/coreclr.git/tests#

* After applying the patch:
=======================
     Test Results (Release Build + O3)
=======================
# CoreCLR Bin Dir  :
/unit-test/bin.coreclr.release.20160713.1.O3.memcopy/Product/Linux.arm.Release
# Tests Discovered : 9873
# Passed           : 9465
# Failed           : 47 (commit: 20160627-52570683, OOM: 2,  manual kill
commands: 0 ) <==== HERE!!!
# Skipped          : 361
=======================
117 minutes and 47 seconds taken to run CoreCLR tests.

real 119m25.213s
user 146m25.355s
sys 16m38.915s
root at arm-target:/unit-test/coreclr.git/tests#

BRs,
Geunsik Lim.

On Mon, Jul 18, 2016 at 3:35 PM, Geunsik Lim <leemgs at gmail.com> wrote:

> > Presumably CoreCLR builds with -Werror,
> Right.
>
> > But there's really no reason to use -O4 in the first place since it's
> identical to -O3.
> Actually, I thought that -O4 means "link-time optimization" based on -O3  (e.g.,
> -O3 + -flto ).
>
>
> > When I grepped CoreCLR I saw rather
> > more uses of UNALIGNED than the two functions you've patched.
> Yes, So, It seems that we have to add an exception handling  to proceed
> arm-specific condition in case of -O3(clang) as following:
>
> ---
>  src/jit/compiler.hpp | 8 ++++----
>  src/pal/inc/pal.h    | 6 ++++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp
> index 7a16b7c..8cee77e 100644
> --- a/src/jit/compiler.hpp
> +++ b/src/jit/compiler.hpp
> @@ -757,16 +757,16 @@ inline
>    signed __int16    getI2LittleEndian(const BYTE * ptr)
>  { return * (UNALIGNED signed __int16 *)ptr; }
>
> -typedef signed __int32 __attribute__((aligned(1))) __unaligned_arm_int32;
> -typedef signed __int64 __attribute__((aligned(1))) __unaligned_arm_int64;
> +typedef signed __int32 UNALIGNED_ARM __unaligned_int32;
> +typedef signed __int64 UNALIGNED_ARM __unaligned_int64;
>
>  inline
>    signed __int32    getI4LittleEndian(const BYTE * ptr)
> -{ return *(__unaligned_arm_int32 *)ptr; }
> +{ return *(__unaligned_int32 *)ptr; }
>
>  inline
>    signed __int64    getI8LittleEndian(const BYTE * ptr)
> -{ return *(__unaligned_arm_int64 *)ptr; }
> +{ return *(__unaligned_int64 *)ptr; }
>
>  inline
>  float               getR4LittleEndian(const BYTE * ptr)
> diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h
> index 5b44661..b95d74c 100644
> --- a/src/pal/inc/pal.h
> +++ b/src/pal/inc/pal.h
> @@ -201,7 +201,13 @@ extern "C" {
>
>  #else // _MSC_VER
>
> +#if defined (_M_ARM) && !defined (DEBUG)
>  #define UNALIGNED
> +#define UNALIGNED_ARM     __attribute__((aligned(1))) //for -O2 and -O3
> +#else
> +#define UNALIGNED
> +#define UNALIGNED_ARM  //for -O0
> +#endif
>
>  #endif // _MSC_VER
>
> --
> 1.9.1
>
>
> >> Do I have to compare the generated instructions to find appropriate
> patch?
> > I'd expect them to be equivalent after optimization
> Do you mean that we get the similar instructions between "memcpy()"
>  library call
> and " __attribute__(aligned(1)) keyword".
>
> Thanks.
>
>
> BRs,
> Geunsik Lim.
>
>
>
> On Fri, Jul 15, 2016 at 12:00 PM, Tim Northover <t.p.northover at gmail.com>
> wrote:
>
>> On 14 July 2016 at 19:47, Geunsik Lim <leemgs at gmail.com> wrote:
>> >> It won't give you an error (just a warning), but no-one I know thinks
>> >>  doing anything special with it is a good idea. It's accepted purely
>> >> for compatibility I think.
>> >
>> > When I used -O3, I have got the below messages while building CoreCLR.
>>
>> Presumably CoreCLR builds with -Werror, which turns any warning into
>> an error. But there's really no reason to use -O4 in the first place
>> since it's identical to -O3.
>>
>> > I am not sure which solution is better between the below two patches.
>> > Could you tell me comment to get the best way?
>>
>> A lot of it comes down to personal preference. Both approaches are
>> functional but you'd have to ask the CoreCLR developers which they
>> prefer.
>>
>> One thing to be wary of is that those patches look like very partial
>> fixes. They may get rid of the current segfault, but leave the
>> underlying problem open in other places (just waiting for some minor
>> change in Clang to trigger it). When I grepped CoreCLR I saw rather
>> more uses of UNALIGNED than the two functions you've patched.
>>
>> > Do I have to compare the generated instructions to find appropriate
>> patch?
>>
>> I'd expect them to be equivalent after optimization.
>>
>> Tim.
>>
>
>
>
> --
> http://leemgs.fedorapeople.org
> Don't try to avoid pain if you fail.
> If you decided to face the challenges in life,
> you can gain a lot by giving your best.
> Cheolsang Jeong's Book & life
> --
>

-- 
http://leemgs.fedorapeople.org
Don't try to avoid pain if you fail.
If you decided to face the challenges in life,
you can gain a lot by giving your best.
Cheolsang Jeong's Book & life
--
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160719/70a204de/attachment.html>