diff mbox

[1/2] Libsanitizer merge from upstream r253555.

Message ID 56542184.5040003@partner.samsung.com
State New
Headers show

Commit Message

max Nov. 24, 2015, 8:36 a.m. UTC 
On 24/11/15 11:25, Jakub Jelinek wrote:
> On Tue, Nov 24, 2015 at 10:51:05AM +0300, Maxim Ostapenko wrote:
>> Ok, I posted a fix to upstream (http://reviews.llvm.org/D14921) yesterday,
>> but it's still not reviewed. So, I'm wondering if I should fix the issue
>> locally?
>> Attaching proposed fix following Jakub's suggestion.
>>
>> Christophe could you try the patch?
>> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
>> index b97fc7d..c392c57 100644
>> --- a/libsanitizer/ChangeLog
>> +++ b/libsanitizer/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>> +
>> +	* include/system/linux/asm/ptrace.h: New header.
>> +
>>   2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>   
>>   	* All source files: Merge from upstream r253555.
>> diff --git a/libsanitizer/include/system/linux/asm/ptrace.h b/libsanitizer/include/system/linux/asm/ptrace.h
>> new file mode 100644
>> index 0000000..dbdd58b
>> --- /dev/null
>> +++ b/libsanitizer/include/system/linux/asm/ptrace.h
>> @@ -0,0 +1,8 @@
>> +#include_next <linux/asm/ptrace.h>
>> +#if defined(__arm__)
>> +#ifndef ARM_VFPREGS_SIZE
>> +/* The size of the user-visible VFP state as seen by PTRACE_GET/SETVFPREGS
>> +   and core dumps.  */
>> +#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
>> +#endif
>> +#endif
> You could have just used #if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
> or #ifdef __arm__
> or #if !defined(ARM_VFPREGS_SIZE).
> Mixing if defined with ifndef on the next line is just weird.
> And, you should mention which kernel version introduced ARM_VFPREGS_SIZE
> macro (I believe it was 2011-ish, but have not checked exact kernel
> version).
>
> 	Jakub
Ok, does it look better now?

-Maxim

Comments

Jakub Jelinek Nov. 24, 2015, 8:38 a.m. UTC | #1 
On Tue, Nov 24, 2015 at 11:36:20AM +0300, Maxim Ostapenko wrote:
> Ok, does it look better now?

Sure, this is ok for trunk.

> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
> index b97fc7d..c392c57 100644
> --- a/libsanitizer/ChangeLog
> +++ b/libsanitizer/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
> +
> +	* include/system/linux/asm/ptrace.h: New header.
> +
>  2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>  
>  	* All source files: Merge from upstream r253555.
> diff --git a/libsanitizer/include/system/linux/asm/ptrace.h b/libsanitizer/include/system/linux/asm/ptrace.h
> new file mode 100644
> index 0000000..d4249fe
> --- /dev/null
> +++ b/libsanitizer/include/system/linux/asm/ptrace.h
> @@ -0,0 +1,7 @@
> +#include_next <linux/asm/ptrace.h>
> +/* ARM_VFPREGS_SIZE has been added in 3.0 */
> +#if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
> +/* The size of the user-visible VFP state as seen by PTRACE_GET/SETVFPREGS
> +   and core dumps.  */
> +#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
> +#endif

	Jakub
max Nov. 24, 2015, 8:58 a.m. UTC | #2 
On 24/11/15 11:38, Jakub Jelinek wrote:
> On Tue, Nov 24, 2015 at 11:36:20AM +0300, Maxim Ostapenko wrote:
>> Ok, does it look better now?
> Sure, this is ok for trunk.
>
>> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
>> index b97fc7d..c392c57 100644
>> --- a/libsanitizer/ChangeLog
>> +++ b/libsanitizer/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>> +
>> +	* include/system/linux/asm/ptrace.h: New header.
>> +
>>   2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>   
>>   	* All source files: Merge from upstream r253555.
>> diff --git a/libsanitizer/include/system/linux/asm/ptrace.h b/libsanitizer/include/system/linux/asm/ptrace.h
>> new file mode 100644
>> index 0000000..d4249fe
>> --- /dev/null
>> +++ b/libsanitizer/include/system/linux/asm/ptrace.h
>> @@ -0,0 +1,7 @@
>> +#include_next <linux/asm/ptrace.h>
>> +/* ARM_VFPREGS_SIZE has been added in 3.0 */
>> +#if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
>> +/* The size of the user-visible VFP state as seen by PTRACE_GET/SETVFPREGS
>> +   and core dumps.  */
>> +#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
>> +#endif
> 	Jakub
>

Thanks, applied as r230790.

Christophe, please let me know if it cured your build.

-Maxim
Christophe Lyon Nov. 24, 2015, 9:21 a.m. UTC | #3 
On 24 November 2015 at 09:58, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> On 24/11/15 11:38, Jakub Jelinek wrote:
>>
>> On Tue, Nov 24, 2015 at 11:36:20AM +0300, Maxim Ostapenko wrote:
>>>
>>> Ok, does it look better now?
>>
>> Sure, this is ok for trunk.
>>
>>> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
>>> index b97fc7d..c392c57 100644
>>> --- a/libsanitizer/ChangeLog
>>> +++ b/libsanitizer/ChangeLog
>>> @@ -1,3 +1,7 @@
>>> +2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>> +
>>> +       * include/system/linux/asm/ptrace.h: New header.
>>> +
>>>   2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>>         * All source files: Merge from upstream r253555.
>>> diff --git a/libsanitizer/include/system/linux/asm/ptrace.h
>>> b/libsanitizer/include/system/linux/asm/ptrace.h
>>> new file mode 100644
>>> index 0000000..d4249fe
>>> --- /dev/null
>>> +++ b/libsanitizer/include/system/linux/asm/ptrace.h
>>> @@ -0,0 +1,7 @@
>>> +#include_next <linux/asm/ptrace.h>
>>> +/* ARM_VFPREGS_SIZE has been added in 3.0 */
>>> +#if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
>>> +/* The size of the user-visible VFP state as seen by
>>> PTRACE_GET/SETVFPREGS
>>> +   and core dumps.  */
>>> +#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
>>> +#endif
>>
>>         Jakub
>>
>
> Thanks, applied as r230790.
>
> Christophe, please let me know if it cured your build.
>

Sure.
I had a build in progress with your proposed patch, but it didn't
complete before you committed :-)

> -Maxim
Christophe Lyon Nov. 24, 2015, 11:08 a.m. UTC | #4 
On 24 November 2015 at 10:21, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 24 November 2015 at 09:58, Maxim Ostapenko
> <m.ostapenko@partner.samsung.com> wrote:
>> On 24/11/15 11:38, Jakub Jelinek wrote:
>>>
>>> On Tue, Nov 24, 2015 at 11:36:20AM +0300, Maxim Ostapenko wrote:
>>>>
>>>> Ok, does it look better now?
>>>
>>> Sure, this is ok for trunk.
>>>
>>>> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
>>>> index b97fc7d..c392c57 100644
>>>> --- a/libsanitizer/ChangeLog
>>>> +++ b/libsanitizer/ChangeLog
>>>> @@ -1,3 +1,7 @@
>>>> +2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>>> +
>>>> +       * include/system/linux/asm/ptrace.h: New header.
>>>> +
>>>>   2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
>>>>         * All source files: Merge from upstream r253555.
>>>> diff --git a/libsanitizer/include/system/linux/asm/ptrace.h
>>>> b/libsanitizer/include/system/linux/asm/ptrace.h
>>>> new file mode 100644
>>>> index 0000000..d4249fe
>>>> --- /dev/null
>>>> +++ b/libsanitizer/include/system/linux/asm/ptrace.h
>>>> @@ -0,0 +1,7 @@
>>>> +#include_next <linux/asm/ptrace.h>
>>>> +/* ARM_VFPREGS_SIZE has been added in 3.0 */
>>>> +#if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
>>>> +/* The size of the user-visible VFP state as seen by
>>>> PTRACE_GET/SETVFPREGS
>>>> +   and core dumps.  */
>>>> +#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
>>>> +#endif
>>>
>>>         Jakub
>>>
>>
>> Thanks, applied as r230790.
>>
>> Christophe, please let me know if it cured your build.
>>
>
> Sure.
> I had a build in progress with your proposed patch, but it didn't
> complete before you committed :-)
>

So... it still does not work for me. I re-tried several times, made sure I had
everything cleanup before starting new builds from scratch, hence the delay.

I'm still seeing:
/tmp/2050111_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:326:44:
error: 'ARM_VFPREGS_SIZE' was not declared in this scope
   unsigned struct_user_vfpregs_struct_sz = ARM_VFPREGS_SIZE;


>> -Maxim
Jakub Jelinek Nov. 24, 2015, 11:12 a.m. UTC | #5 
On Tue, Nov 24, 2015 at 12:08:13PM +0100, Christophe Lyon wrote:
> > Sure.
> > I had a build in progress with your proposed patch, but it didn't
> > complete before you committed :-)
> >
> 
> So... it still does not work for me. I re-tried several times, made sure I had
> everything cleanup before starting new builds from scratch, hence the delay.
> 
> I'm still seeing:
> /tmp/2050111_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:326:44:
> error: 'ARM_VFPREGS_SIZE' was not declared in this scope
>    unsigned struct_user_vfpregs_struct_sz = ARM_VFPREGS_SIZE;

So cut'n'paste the sanitizer_platform_limits_posix.cc compilation command
line and replace -c with -E -dD, then look if the wrapper asm/ptrace.h is
included or not and why?

	Jakub
Christophe Lyon Nov. 24, 2015, 11:23 a.m. UTC | #6 
On 24 November 2015 at 12:12, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 24, 2015 at 12:08:13PM +0100, Christophe Lyon wrote:
>> > Sure.
>> > I had a build in progress with your proposed patch, but it didn't
>> > complete before you committed :-)
>> >
>>
>> So... it still does not work for me. I re-tried several times, made sure I had
>> everything cleanup before starting new builds from scratch, hence the delay.
>>
>> I'm still seeing:
>> /tmp/2050111_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:326:44:
>> error: 'ARM_VFPREGS_SIZE' was not declared in this scope
>>    unsigned struct_user_vfpregs_struct_sz = ARM_VFPREGS_SIZE;
>
> So cut'n'paste the sanitizer_platform_limits_posix.cc compilation command
> line and replace -c with -E -dD, then look if the wrapper asm/ptrace.h is
> included or not and why?
>
It pulls the one from the sysroot:
sysroot-arm-none-linux-gnueabihf/usr/include/asm/ptrace.h
(I configure GCC --with-sysroot=XXX)

>         Jakub
Jakub Jelinek Nov. 24, 2015, 11:27 a.m. UTC | #7 
On Tue, Nov 24, 2015 at 12:23:05PM +0100, Christophe Lyon wrote:
> On 24 November 2015 at 12:12, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, Nov 24, 2015 at 12:08:13PM +0100, Christophe Lyon wrote:
> >> > Sure.
> >> > I had a build in progress with your proposed patch, but it didn't
> >> > complete before you committed :-)
> >> >
> >>
> >> So... it still does not work for me. I re-tried several times, made sure I had
> >> everything cleanup before starting new builds from scratch, hence the delay.
> >>
> >> I'm still seeing:
> >> /tmp/2050111_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:326:44:
> >> error: 'ARM_VFPREGS_SIZE' was not declared in this scope
> >>    unsigned struct_user_vfpregs_struct_sz = ARM_VFPREGS_SIZE;
> >
> > So cut'n'paste the sanitizer_platform_limits_posix.cc compilation command
> > line and replace -c with -E -dD, then look if the wrapper asm/ptrace.h is
> > included or not and why?
> >
> It pulls the one from the sysroot:
> sysroot-arm-none-linux-gnueabihf/usr/include/asm/ptrace.h
> (I configure GCC --with-sysroot=XXX)

Then you should figure out where the sysroot include dirs are added in the
sanitizer_common/Makefile and make sure -isystem $(top_srcdir)/include/system
comes before that.

	Jakub
diff mbox

Patch

diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
index b97fc7d..c392c57 100644
--- a/libsanitizer/ChangeLog
+++ b/libsanitizer/ChangeLog
@@ -1,3 +1,7 @@ 
+2015-11-24  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
+
+	* include/system/linux/asm/ptrace.h: New header.
+
 2015-11-23  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>
 
 	* All source files: Merge from upstream r253555.
diff --git a/libsanitizer/include/system/linux/asm/ptrace.h b/libsanitizer/include/system/linux/asm/ptrace.h
new file mode 100644
index 0000000..d4249fe
--- /dev/null
+++ b/libsanitizer/include/system/linux/asm/ptrace.h
@@ -0,0 +1,7 @@ 
+#include_next <linux/asm/ptrace.h>
+/* ARM_VFPREGS_SIZE has been added in 3.0 */
+#if defined(__arm__) && !defined(ARM_VFPREGS_SIZE)
+/* The size of the user-visible VFP state as seen by PTRACE_GET/SETVFPREGS
+   and core dumps.  */
+#define ARM_VFPREGS_SIZE ( 32 * 8 /*fpregs*/ + 4 /*fpscr*/ )
+#endif