Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

[Sergio Durigan Junior]

On Sunday, October 01 2017, gdb-buildbot@sergiodj.net wrote:

> My lords, ladies, gentlemen, members of the public.
>
> It is a matter of great regret and sadness to inform you that commit:
>
> 	Use a std::vector for ada_exceptions_list
> 	ab816a274505933da2f854014b54901c3c3db9d2
>
> might have made GDB unwell.  Since I am just your Butler BuildBot,
> I kindly ask that a human superior officer double-check this.
>
> Please note that if you are reading this message on gdb-patches, there might
> be other builders broken.
>
> You can find more details about the unfortunate breakage in the next messages.

FWIW, the brekages happened on RHEL-s390x-m64, Ubuntu-AArch64-m64 and
Ubuntu-AArch64-native-gdbserver-m64 (so far).

The compilation log says:

In file included from /usr/include/c++/4.8/algorithm:62:0,
                 from ../../binutils-gdb/gdb/ada-lang.c:65:
/usr/include/c++/4.8/bits/stl_algo.h: In instantiation of ‘_RandomAccessIterator std::__unguarded_partition(_RandomAccessIterator, _RandomAccessIterator, const _Tp&) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<ada_exc_info*, std::vector<ada_exc_info> >; _Tp = ada_exc_info]’:
/usr/include/c++/4.8/bits/stl_algo.h:2283:70:   required from ‘_RandomAccessIterator std::__unguarded_partition_pivot(_RandomAccessIterator, _RandomAccessIterator) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<ada_exc_info*, std::vector<ada_exc_info> >]’
/usr/include/c++/4.8/bits/stl_algo.h:2315:54:   required from ‘void std::__introsort_loop(_RandomAccessIterator, _RandomAccessIterator, _Size) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<ada_exc_info*, std::vector<ada_exc_info> >; _Size = long int]’
/usr/include/c++/4.8/bits/stl_algo.h:5461:36:   required from ‘void std::sort(_RAIter, _RAIter) [with _RAIter = __gnu_cxx::__normal_iterator<ada_exc_info*, std::vector<ada_exc_info> >]’
../../binutils-gdb/gdb/ada-lang.c:13153:61:   required from here
/usr/include/c++/4.8/bits/stl_algo.h:2245:19: error: passing ‘const ada_exc_info’ as ‘this’ argument of ‘bool ada_exc_info::operator<(const ada_exc_info&)’ discards qualifiers [-fpermissive]
    while (__pivot < *__last)
                   ^
make[2]: *** [ada-lang.o] Error 1

Cheers,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> /usr/include/c++/4.8/bits/stl_algo.h:2245:19: error: passing ‘const ada_exc_info’ as ‘this’ argument of ‘bool ada_exc_info::operator<(const ada_exc_info&)’ discards qualifiers [-fpermissive]

I don't really understand this error, and especially why I don't see it
(and didn't see it from try?), but I wonder if changing operator< and
operator== to const-qualified would help.

The appended is an attempt but I'm not sure how to test it now.

Tom

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 7e9f06c..1a0c769 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13123,7 +13123,7 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
    by exception address.  */
 
 bool
-ada_exc_info::operator< (const ada_exc_info &other)
+ada_exc_info::operator< (const ada_exc_info &other) const
 {
   int result;
 
@@ -13136,7 +13136,7 @@ ada_exc_info::operator< (const ada_exc_info &other)
 }
 
 bool
-ada_exc_info::operator== (const ada_exc_info &other)
+ada_exc_info::operator== (const ada_exc_info &other) const
 {
   return addr == other.addr && strcmp (name, other.name) == 0;
 }
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index f92b88d..a47fe82 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -387,8 +387,8 @@ struct ada_exc_info
   /* The address of the symbol corresponding to that exception.  */
   CORE_ADDR addr;
 
-  bool operator< (const ada_exc_info &);
-  bool operator== (const ada_exc_info &);
+  bool operator< (const ada_exc_info &) const;
+  bool operator== (const ada_exc_info &) const;
 };
 
 extern std::vector<ada_exc_info> ada_exceptions_list (const char *regexp);

Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

[Sergio Durigan Junior]

On Sunday, October 01 2017, Tom Tromey wrote:

>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
> Sergio> /usr/include/c++/4.8/bits/stl_algo.h:2245:19: error: passing
> Sergio> ‘const ada_exc_info’ as ‘this’ argument of ‘bool
> Sergio> ada_exc_info::operator<(const ada_exc_info&)’ discards
> Sergio> qualifiers [-fpermissive]
>
> I don't really understand this error, and especially why I don't see it
> (and didn't see it from try?), but I wonder if changing operator< and
> operator== to const-qualified would help.

Hm, it may be that you're not testing your patches on the AArch64
builders, maybe?  At least I don't see any try builds from you here:

  https://gdb-build.sergiodj.net/builders/Ubuntu-AArch64-m64?numbuilds=500

> The appended is an attempt but I'm not sure how to test it now.

You can test by specifying the AArch64 builder manually via the cli:

  buildbot try -b Ubuntu-AArch64-m64

This should run the try build only on the specified builder.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

[Tejas Belagod]

On 02/10/17 03:43, Sergio Durigan Junior wrote:
> On Sunday, October 01 2017, Tom Tromey wrote:
>
>>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>>
>> Sergio> /usr/include/c++/4.8/bits/stl_algo.h:2245:19: error: passing
>> Sergio> ‘const ada_exc_info’ as ‘this’ argument of ‘bool
>> Sergio> ada_exc_info::operator<(const ada_exc_info&)Â’ discards
>> Sergio> qualifiers [-fpermissive]
>>
>> I don't really understand this error, and especially why I don't see it
>> (and didn't see it from try?), but I wonder if changing operator< and
>> operator== to const-qualified would help.
>
> Hm, it may be that you're not testing your patches on the AArch64
> builders, maybe?  At least I don't see any try builds from you here:
>
>    https://gdb-build.sergiodj.net/builders/Ubuntu-AArch64-m64?numbuilds=500
>
>> The appended is an attempt but I'm not sure how to test it now.
>
> You can test by specifying the AArch64 builder manually via the cli:
>
>    buildbot try -b Ubuntu-AArch64-m64
>
> This should run the try build only on the specified builder.
>

Tom,

This also fails to build for arm-none-eabi

$ src/gdb/configure --target=arm-none-eabi --disable-nls --disable-sim 
--disable-gas --disable-binutils --disable-ld --disable-gprof --with-libexpat 
--with-lzma=no 
--with-system-gdbinit=..../install-native/x86_64-linux-gnu/arm-none-eabi/lib/gdbinit 
--build=x86_64-linux-gnu --host=x86_64-linux-gnu 
--with-libexpat-prefix=..../build-native/host-libs/usr --with-python=no 
'--with-gdb-datadir='\''${prefix}'\''/arm-none-eabi/share/gdb' && make -j24

Thanks,
Tejas.

Re: Oh dear. I regret to inform you that commit ab816a274505933da2f854014b54901c3c3db9d2 might be unfortunate

[Pedro Alves]

On 10/02/2017 03:04 AM, Tom Tromey wrote:
>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
> 
> Sergio> /usr/include/c++/4.8/bits/stl_algo.h:2245:19: error: passing ‘const ada_exc_info’ as ‘this’ argument of ‘bool ada_exc_info::operator<(const ada_exc_info&)’ discards qualifiers [-fpermissive]
> 
> I don't really understand this error, 

I have a local build of gcc 4.8.5, and I see the same.  The const is
coming from here:

/opt/gcc-4.8/include/c++/4.8.5/bits/stl_algo.h: In instantiation of ‘_RandomAccessIterator std::__unguarded_partition(_RandomAccessIterator, _RandomAccessIterator, const _Tp&) [with _RandomAccessIterator = 
                                                                         ^^^^^^^^^^
__gnu_cxx::__normal_iterator<ada_exc_info*, std::vector<ada_exc_info> >; _Tp = ada_exc_info]’:
                                                                         ^^^^^^^^^^^^^^^^^^

I think that was a bug in libstdc++.  std::__unguarded_partition no longer take
'const _Tp&' in the current sources.  Git blame points at:

  https://gcc.gnu.org/ml/libstdc++/2012-04/msg00074.html

where François wrote (emphasis mine):

~~~~
 - __unguarded_partition used to have 3 template parameters: _RandomAccessIterator, _Tp and _Compare. It was taking a const reference 
                                                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 to _Tp which was a useless constraint for the functor. So it now only have 2 template parameters, const _Tp& has been replaced by 
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 iterator_traits<_RandomAccessIterator>::reference.
~~~~

and especially why I don't see it
> (and didn't see it from try?), but I wonder if changing operator< and
> operator== to const-qualified would help.
> 
> The appended is an attempt but I'm not sure how to test it now.

Yes, I also think that that's the right fix.  I've pushed it in now.

Thanks,
Pedro Alves