[llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.

[Jovanovic, Petar]

Hi Eric,

I have just updated the patch to include the comment for the change.

The issue is that code for some switch statements with __builtin_unreachable
is not correctly generated. Some cases miss, and the code is wrong. The wrong
code resulted in wrong values and eventually in null-pointer dereference and
code crash. The issue has been found in Renderscript examples in Android that
crashed due to incorrect llvm library code.

Regards,
Petar
________________________________________
From: Eric Christopher [echristo at apple.com]
Sent: Tuesday, July 24, 2012 10:34 PM
To: Jovanovic, Petar
Cc: llvm-commits at cs.uiuc.edu; Benjamin Kramer
Subject: Re: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.

I'm not seeing any update in the code as to why you're working around it (which was Benjamin's request), and the comment that you made in your reply also doesn't state what the bug was other than "issue".

-eric

On Jul 24, 2012, at 11:46 AM, "Jovanovic, Petar" <petarj at mips.com> wrote:

> PTAL
>
> -----Original Message-----
> From: Petar Jovanovic [mailto:petar.jovanovic at rt-rk.com]
> Sent: Thursday, April 12, 2012 3:15 AM
> To: 'Benjamin Kramer'
> Cc: 'llvm-commits at cs.uiuc.edu'
> Subject: RE: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>
> Any additional comments or concerns about this one?
>
> Regards,
> Petar
>
> -----Original Message-----
> From: Petar Jovanovic [mailto:petar.jovanovic at rt-rk.com]
> Sent: Friday, March 23, 2012 7:15 PM
> To: 'Benjamin Kramer'
> Cc: 'llvm-commits at cs.uiuc.edu'
> Subject: RE: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>
> The issue was first found on the latest cross gcc from Mentor [gcc version 4.5.2 (Sourcery CodeBench Lite 2011.09-75)], and since __builtin_unreachable() is available as of gcc 4.5, it seemed to me that the scope is already narrow enough. I can also confirm that the bug can be reproduced with native gcc (GCC) 4.5.1 and gcc (GCC) 4.5.3 , and I am actually not aware of gcc version that supports __builtin_unreachable in which the bug is not present.
>
> The bug was discovered three days ago in Android builds for MIPS. The function where this issue was discovered is:
> unsigned TargetData::getAlignment(Type *Ty, bool abi_or_pref)
>
> at lib\Target\TargetData.cpp.
>
> Petar
>
> -----Original Message-----
> From: Benjamin Kramer [mailto:benny.kra at googlemail.com]
> Sent: Friday, March 23, 2012 12:57 AM
> To: Petar Jovanovic
> Cc: llvm-commits at cs.uiuc.edu; Petar Jovanovic
> Subject: Re: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>
> On 23.03.2012, at 00:20, Petar Jovanovic <petar.jovanovic at rt-rk.com> wrote:
>
>> There is a GCC bug which can be triggered inside of LLVM library. The -O2 opt
>> does not work well with __builtin_unreachable(), so we are avoiding it inside
>> of LLVM. The issue seems to be caused by 'freorder-blocks' optimization.
>
> Patches working around compiler bugs are generally okay, but please
>
> - Add a comment on why you're blacklisting MIPS here.
> - Make the blacklist as narrow as possible, i.e. check for the
> specific version of GCC that has this bug.
>
> - Ben
>>
>> Signed-off-by: Petar Jovanovic <petarj at mips.com>
>> ---
>> include/llvm/Support/Compiler.h |    2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/llvm/Support/Compiler.h b/include/llvm/Support/Compiler.h
>> index 9c5e7ec..b5a086a 100644
>> --- a/include/llvm/Support/Compiler.h
>> +++ b/include/llvm/Support/Compiler.h
>> @@ -144,9 +144,11 @@
>> // LLVM_BUILTIN_UNREACHABLE - On compilers which support it, expands
>> // to an expression which states that it is undefined behavior for the
>> // compiler to reach this point.  Otherwise is not defined.
>> +#if !defined(__mips__)
>> #if defined(__clang__) || (__GNUC__ > 4) \
>> || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
>> # define LLVM_BUILTIN_UNREACHABLE __builtin_unreachable()
>> #endif
>> +#endif
>>
>> #endif
>> --
>> 1.7.5.4
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
A non-text attachment was scrubbed...
Name: disable-builtin-unreachable.diff
Type: application/octet-stream
Size: 862 bytes
Desc: disable-builtin-unreachable.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120724/09c7ab88/attachment.obj>