[PUSHED] gdb: Use std::abs instead of abs on LONGEST types

Message ID 20200227164651.13723-1-andrew.burgess@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess Feb. 27, 2020, 4:46 p.m. UTC
  Use std::abs so that we get the C++ overloaded version that matches
the argument type instead of the C abs function which is only for int
arguments.

There should be no user visible change after this commit.

gdb/ChangeLog:

	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
	abs.
---
 gdb/ChangeLog  | 5 +++++
 gdb/gdbtypes.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)
  

Comments

Pedro Alves Feb. 27, 2020, 7:06 p.m. UTC | #1
On 2/27/20 4:46 PM, Andrew Burgess wrote:
> Use std::abs so that we get the C++ overloaded version that matches
> the argument type instead of the C abs function which is only for int
> arguments.

Note that stdlib.h/stdmath.h are supposed to provide the overloads in
the global namespace as well; the standard requires it.  Older
GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.

Just a FYI, the patch is fine.

> 
> There should be no user visible change after this commit.
> 
> gdb/ChangeLog:
> 
> 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
> 	abs.

Thanks,
Pedro Alves
  
Terekhov, Mikhail via Gdb-patches Feb. 27, 2020, 7:09 p.m. UTC | #2
On Thu, Feb 27, 2020 at 1:07 PM Pedro Alves <palves@redhat.com> wrote:
>
> On 2/27/20 4:46 PM, Andrew Burgess wrote:
> > Use std::abs so that we get the C++ overloaded version that matches
> > the argument type instead of the C abs function which is only for int
> > arguments.
>
> Note that stdlib.h/stdmath.h are supposed to provide the overloads in
> the global namespace as well; the standard requires it.  Older
> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.
>
> Just a FYI, the patch is fine.

Hm... I saw a build error from this on arm-netbsd with clang 9, I
wonder what happened there. Anyway, the patch does fix it.

Christian

>
> >
> > There should be no user visible change after this commit.
> >
> > gdb/ChangeLog:
> >
> >       * gdbtypes.c (create_array_type_with_stride): Use std::abs not
> >       abs.
>
> Thanks,
> Pedro Alves
>
  
Pedro Alves Feb. 27, 2020, 7:26 p.m. UTC | #3
On 2/27/20 7:09 PM, Christian Biesinger via gdb-patches wrote:
> On Thu, Feb 27, 2020 at 1:07 PM Pedro Alves <palves@redhat.com> wrote:
>>
>> On 2/27/20 4:46 PM, Andrew Burgess wrote:
>>> Use std::abs so that we get the C++ overloaded version that matches
>>> the argument type instead of the C abs function which is only for int
>>> arguments.
>>
>> Note that stdlib.h/stdmath.h are supposed to provide the overloads in
>> the global namespace as well; the standard requires it.  Older
>> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.
>>
>> Just a FYI, the patch is fine.
> 
> Hm... I saw a build error from this on arm-netbsd with clang 9, I
> wonder what happened there. Anyway, the patch does fix it.
> 

( See:
 https://developers.redhat.com/blog/2016/02/29/why-cstdlib-is-more-complicated-than-you-might-think/ )

Odd, clang 5, which is what I have handy, gets it right:

$ cat abs.cc 
#include <stdlib.h>
#include <stdio.h>

void
foo (long i)
{
  printf ("long\n");
}

void
foo (int i)
{
  printf ("int\n");
}

int
main ()
{
  foo (abs ((long)1));
  foo (abs ((int)1));
}

$ clang++ abs.cc -o abs && ./abs
long
int

I wonder whether you were seeing a gnulib override issue, but I
can't find an abs override in our import.

Thanks,
Pedro Alves
  
Andrew Burgess Feb. 27, 2020, 9:10 p.m. UTC | #4
* Pedro Alves <palves@redhat.com> [2020-02-27 19:06:47 +0000]:

> On 2/27/20 4:46 PM, Andrew Burgess wrote:
> > Use std::abs so that we get the C++ overloaded version that matches
> > the argument type instead of the C abs function which is only for int
> > arguments.
> 
> Note that stdlib.h/stdmath.h are supposed to provide the overloads in
> the global namespace as well; the standard requires it.  Older
> GCCs got that wrong (e.g. 4.8), but more modern GCCs get it right.
> 
> Just a FYI, the patch is fine.


Thanks for looking at this.

Thanks,
Andrew




> 
> > 
> > There should be no user visible change after this commit.
> > 
> > gdb/ChangeLog:
> > 
> > 	* gdbtypes.c (create_array_type_with_stride): Use std::abs not
> > 	abs.
> 
> Thanks,
> Pedro Alves
>
  

Patch

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index ef110b30445..d89df9f7409 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1249,9 +1249,9 @@  create_array_type_with_stride (struct type *result_type,
 	     negative stride in Fortran (this doesn't mean anything
 	     special, it's still just a single element array) so do
 	     consider that case when touching this code.  */
-	  LONGEST element_count = abs (high_bound - low_bound + 1);
+	  LONGEST element_count = std::abs (high_bound - low_bound + 1);
 	  TYPE_LENGTH (result_type)
-	    = ((abs (stride) * element_count) + 7) / 8;
+	    = ((std::abs (stride) * element_count) + 7) / 8;
 	}
       else
 	TYPE_LENGTH (result_type) =