[PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

Hi,

The gdb.base/dump.exp test had a few failures for MIPS N64. That is 
expected since SREC, IHEX and TEKHEX dump formats don't support 64-bit 
addresses, and MIPS N64 uses them.

Skipping this test for MIPS is not the right thing as some ABI's are 
32-bit and so should be tested against the usual set of SREC, IHEX and 
TEKHEX formats.

The attached patch checks for the N64 ABI and sets the internal flag 
is64bitonly accordingly, skipping the SREC, IHEX and TEKHEX tests.

Ok?

[-- Attachment #2: dump_mips.diff --]
[-- Type: text/x-patch, Size: 1048 bytes --]

2013-07-01  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/dump.exp: Don't run SREC, IHEX and TEKHEX dump
	tests for mips N64 ABI.

Index: gdb/testsuite/gdb.base/dump.exp
===================================================================
--- gdb/testsuite/gdb.base/dump.exp	(revision 415809)
+++ gdb/testsuite/gdb.base/dump.exp	(working copy)
@@ -58,6 +58,17 @@
 
 gdb_load ${binfile}
 
+# Since SREC, IHEX and TEKHEX cannot handle 64-bit addresses for
+# MIPS64, stop testing if we have a 64-bit MIPS program.
+if {[istarget "mips*-*-*"]} then {
+  send_gdb "show mips abi\n"
+  gdb_expect {
+      -re ".*n64.*$gdb_prompt $" {
+	  set is64bitonly "yes"
+      }
+  }
+}
+
 # Clean up any stale output files from previous test runs
 
 remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex intstr2.srec intstr2.tekhex intarr3.srec"

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Yao Qi]

On 07/02/2013 12:24 AM, Luis Machado wrote:
> +# Since SREC, IHEX and TEKHEX cannot handle 64-bit addresses for
> +# MIPS64, stop testing if we have a 64-bit MIPS program.
> +if {[istarget "mips*-*-*"]} then {
> +  send_gdb "show mips abi\n"
> +  gdb_expect {
> +      -re ".*n64.*$gdb_prompt $" {
> +	  set is64bitonly "yes"
> +      }
> +  }
> +}
> +

Luis,
this patch looks reasonable to me.  Probably, we should move the code to 
the place close to the existing code setting "is64bitonly" too.

-- 
Yao (齐尧)

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/02/2013 11:37 AM, Yao Qi wrote:
> On 07/02/2013 12:24 AM, Luis Machado wrote:
>> +# Since SREC, IHEX and TEKHEX cannot handle 64-bit addresses for
>> +# MIPS64, stop testing if we have a 64-bit MIPS program.
>> +if {[istarget "mips*-*-*"]} then {
>> +  send_gdb "show mips abi\n"
>> +  gdb_expect {
>> +      -re ".*n64.*$gdb_prompt $" {
>> +      set is64bitonly "yes"
>> +      }
>> +  }
>> +}
>> +
>
> Luis,
> this patch looks reasonable to me.  Probably, we should move the code to
> the place close to the existing code setting "is64bitonly" too.
>

For MIPS we need to load the binary first to be able to tell if we 
really have a N64 ABI or not. That is why this check is a bit later in 
the code compared to the other checks.

Does that make sense?

Luis

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Yao Qi]

On 07/02/2013 10:45 PM, Luis Machado wrote:
> For MIPS we need to load the binary first to be able to tell if we
> really have a N64 ABI or not. That is why this check is a bit later in
> the code compared to the other checks.
>
> Does that make sense?

Yes, you are right.

-- 
Yao (齐尧)

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Tom Tromey]

>>>>> "Luis" == Luis Machado <lgustavo@codesourcery.com> writes:

Luis> Skipping this test for MIPS is not the right thing as some ABI's are
Luis> 32-bit and so should be tested against the usual set of SREC, IHEX and
Luis> TEKHEX formats.

Luis> The attached patch checks for the N64 ABI and sets the internal flag
Luis> is64bitonly accordingly, skipping the SREC, IHEX and TEKHEX tests.

Should this be made more generic?
Say, test for "sizeof(&main) != 4"?

Tom

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/02/2013 01:47 PM, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado <lgustavo@codesourcery.com> writes:
>
> Luis> Skipping this test for MIPS is not the right thing as some ABI's are
> Luis> 32-bit and so should be tested against the usual set of SREC, IHEX and
> Luis> TEKHEX formats.
>
> Luis> The attached patch checks for the N64 ABI and sets the internal flag
> Luis> is64bitonly accordingly, skipping the SREC, IHEX and TEKHEX tests.
>
> Should this be made more generic?
> Say, test for "sizeof(&main) != 4"?

It could be, if we can guarantee oddball architectures don't use 
pointers and addresses of strange sizes, say, an arch that declares 
function pointers of 4 bytes.

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Stan Shebs]

On 7/2/13 9:51 AM, Luis Machado wrote:
> On 07/02/2013 01:47 PM, Tom Tromey wrote:
>>>>>>> "Luis" == Luis Machado <lgustavo@codesourcery.com> writes:
>>
>> Luis> Skipping this test for MIPS is not the right thing as some ABI's
>> are
>> Luis> 32-bit and so should be tested against the usual set of SREC,
>> IHEX and
>> Luis> TEKHEX formats.
>>
>> Luis> The attached patch checks for the N64 ABI and sets the internal
>> flag
>> Luis> is64bitonly accordingly, skipping the SREC, IHEX and TEKHEX tests.
>>
>> Should this be made more generic?
>> Say, test for "sizeof(&main) != 4"?
> 
> It could be, if we can guarantee oddball architectures don't use
> pointers and addresses of strange sizes, say, an arch that declares
> function pointers of 4 bytes.

You could test both a function address and a data address, and if either
is greater than 4 bytes, the old formats can be excluded from testing.

Stan
stan@codesourcery.com

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Tom Tromey]

>> It could be, if we can guarantee oddball architectures don't use
>> pointers and addresses of strange sizes, say, an arch that declares
>> function pointers of 4 bytes.

Stan> You could test both a function address and a data address, and if either
Stan> is greater than 4 bytes, the old formats can be excluded from testing.

FWIW I'm actually ok with the original patch as well.
So if Stan is happy with it, it is ok.

Tom

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On 07/02/2013 03:10 PM, Tom Tromey wrote:
>>> It could be, if we can guarantee oddball architectures don't use
>>> pointers and addresses of strange sizes, say, an arch that declares
>>> function pointers of 4 bytes.
>
> Stan> You could test both a function address and a data address, and if either
> Stan> is greater than 4 bytes, the old formats can be excluded from testing.
>
> FWIW I'm actually ok with the original patch as well.
> So if Stan is happy with it, it is ok.

What about this one?

I feel i repeated myself in the comments a little, in an attempt to make 
things clear. It may sound like a broken LP. :-)

Luis


[-- Attachment #2: dump_mips.diff --]
[-- Type: text/x-patch, Size: 2251 bytes --]

2013-07-02  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/dump.exp: Update copyright line.
	Remove arch-specific tests and do a generic pointer size
	check to set is64bitonly correctly.

Index: testsuite/gdb.base/dump.exp
===================================================================
--- testsuite/gdb.base/dump.exp	(revision 415809)
+++ testsuite/gdb.base/dump.exp	(working copy)
@@ -1,4 +1,4 @@
-# Copyright 2002, 2004, 2007-2012 Free Software Foundation, Inc.
+# Copyright 2002, 2004, 2007-2013 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -32,16 +32,6 @@
     lappend options "additional_flags=-Wl,-taso"
 }
 
-if {[istarget "ia64*-*-*"] || [istarget "hppa64-*-*"]} then {
-    set is64bitonly "yes"
-}
-
-if {[istarget "spu*-*-*"]} then {
-    # The internal address format used for the combined Cell/B.E.
-    # debugger requires 64-bit.
-    set is64bitonly "yes"
-}
-
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
      untested dump.exp
      return -1
@@ -58,6 +48,22 @@
 
 gdb_load ${binfile}
 
+# Decide if we should test SREC, IHEX and TEKHEX formats.
+if {![istarget "alpha*-*-*"]} then {
+    # Check the size of a function pointer and of a data pointer.  If
+    # both are different from 4-bytes, we probably have 64-bit
+    # addresses that are not supported by SREC, IHEX and TEKHEX.  We
+    # skip those tests then.
+    # If we error out below, we use the defaults (8 bytes) and skip
+    # the SREC, IHEX and TEKHEX tests just to be safe.
+
+    set sizeof_function_ptr [get_sizeof "void (*)(void)" 8]
+    set sizeof_data_ptr [get_sizeof "void *" 8]
+    if {${sizeof_function_ptr} != 4 && ${sizeof_data_ptr} != 4} then {
+	set is64bitonly "yes"
+    }
+}
+
 # Clean up any stale output files from previous test runs
 
 remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex intstr2.srec intstr2.tekhex intarr3.srec"

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Tom Tromey]

>>>>> "Luis" == Luis Machado <lgustavo@codesourcery.com> writes:

Luis> What about this one?

Luis> 2013-07-02  Luis Machado  <lgustavo@codesourcery.com>

Luis> 	* gdb.base/dump.exp: Update copyright line.
Luis> 	Remove arch-specific tests and do a generic pointer size
Luis> 	check to set is64bitonly correctly.

Ok, thanks.

Tom

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Pedro Alves]

On 07/02/2013 07:50 PM, Luis Machado wrote:
> -
> -if {[istarget "spu*-*-*"]} then {
> -    # The internal address format used for the combined Cell/B.E.
> -    # debugger requires 64-bit.
> -    set is64bitonly "yes"
> -}
> -

I'm not sure this equates to sizeof pointer == 64-bit.
This bit may need to be retained.  [Adding Ulrich].

> +
> +    set sizeof_function_ptr [get_sizeof "void (*)(void)" 8]
> +    set sizeof_data_ptr [get_sizeof "void *" 8]
> +    if {${sizeof_function_ptr} != 4 && ${sizeof_data_ptr} != 4} then {
> +	set is64bitonly "yes"
> +    }
> +}
> +

srec (etc.) is most used in small embedded targets (e.g., those
that include dsrec.o in the configure.tgt), consequently
that's where the test is most useful.  Such targets
are the most likely to have 16-bit pointers (< 4 bytes).
E.g., h8300, etc.  Looks like this ends up causing the tests to
be skipped there too.  IOW, a better check would be:

   if {${sizeof_function_ptr} > 4 || ${sizeof_data_ptr} > 4} then {

But, this change also means we have reduced routine-checking,
as most people test on x86_64.  I think we can do better.  The test
works fine on e.g., x86_64, because programs get linked to low (< 32-bit)
addresses by default.  That's the point of:

if [istarget "alpha*-*-*"] then {
    # SREC etc cannot handle 64-bit addresses.  Force the test
    # program into the low 31 bits of the address space.
    lappend options "additional_flags=-Wl,-taso"
}

(For MIPS N64, if you wanted, I guess you could do similarly
 to Alpha, and rebuild with:

  lappend options "ldflags=-Wl,-Tdata=0x600000"

 to force use of low addresses.)

IOW, instead of checking for ABI pointer sizes, I think it'd
be better to test for the actual address size of one the
variables dumped.  That is, check that &intarray is < 32-bit.

-- 
Pedro Alves

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 2504 bytes --]

On 07/03/2013 12:05 PM, Pedro Alves wrote:
> On 07/02/2013 07:50 PM, Luis Machado wrote:
>> -
>> -if {[istarget "spu*-*-*"]} then {
>> -    # The internal address format used for the combined Cell/B.E.
>> -    # debugger requires 64-bit.
>> -    set is64bitonly "yes"
>> -}
>> -
>
> I'm not sure this equates to sizeof pointer == 64-bit.
> This bit may need to be retained.  [Adding Ulrich].

Fair enough. Ulrich, let me know if the pointer check in the attached 
patch doesn't make sense for Cell BE.

>
>> +
>> +    set sizeof_function_ptr [get_sizeof "void (*)(void)" 8]
>> +    set sizeof_data_ptr [get_sizeof "void *" 8]
>> +    if {${sizeof_function_ptr} != 4 && ${sizeof_data_ptr} != 4} then {
>> +	set is64bitonly "yes"
>> +    }
>> +}
>> +
>
> srec (etc.) is most used in small embedded targets (e.g., those
> that include dsrec.o in the configure.tgt), consequently
> that's where the test is most useful.  Such targets
> are the most likely to have 16-bit pointers (< 4 bytes).
> E.g., h8300, etc.  Looks like this ends up causing the tests to
> be skipped there too.  IOW, a better check would be:
>
>     if {${sizeof_function_ptr} > 4 || ${sizeof_data_ptr} > 4} then {
>

Ah, yes. This check is indeed better. Follows an updated patch that does 
this.

> But, this change also means we have reduced routine-checking,
> as most people test on x86_64.  I think we can do better.  The test
> works fine on e.g., x86_64, because programs get linked to low (< 32-bit)
> addresses by default.  That's the point of:
>
> if [istarget "alpha*-*-*"] then {
>      # SREC etc cannot handle 64-bit addresses.  Force the test
>      # program into the low 31 bits of the address space.
>      lappend options "additional_flags=-Wl,-taso"
> }
>
> (For MIPS N64, if you wanted, I guess you could do similarly
>   to Alpha, and rebuild with:
>
>    lappend options "ldflags=-Wl,-Tdata=0x600000"
>
>   to force use of low addresses.)
>
> IOW, instead of checking for ABI pointer sizes, I think it'd
> be better to test for the actual address size of one the
> variables dumped.  That is, check that &intarray is < 32-bit.
>

If lack of coverage for x86_64 running things on low addresses is a 
problem, we can add an exception for x86_64, what do you think? Adding 
these exceptions usually polute the testcases though.

As for MIPS, attempting to force the use of low addresses, just like 
alpha, seems to do more than what the tools expect at the moment, and i 
get a SIGSEGV in the dynamic loader.

Luis

[-- Attachment #2: dump.diff --]
[-- Type: text/x-patch, Size: 1978 bytes --]

2013-07-03  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/dump.exp: Update copyright line.
	Remove arch-specific tests and do a generic pointer size
	check to set is64bitonly correctly.

Index: gdb/testsuite/gdb.base/dump.exp
===================================================================
--- gdb/testsuite/gdb.base/dump.exp	(revision 415997)
+++ gdb/testsuite/gdb.base/dump.exp	(working copy)
@@ -32,16 +32,6 @@
     lappend options "additional_flags=-Wl,-taso"
 }
 
-if {[istarget "ia64*-*-*"] || [istarget "hppa64-*-*"]} then {
-    set is64bitonly "yes"
-}
-
-if {[istarget "spu*-*-*"]} then {
-    # The internal address format used for the combined Cell/B.E.
-    # debugger requires 64-bit.
-    set is64bitonly "yes"
-}
-
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
      untested dump.exp
      return -1
@@ -58,6 +48,23 @@
 
 gdb_load ${binfile}
 
+# Decide if we should test SREC, IHEX and TEKHEX formats.
+if {![istarget "alpha*-*-*"]} then {
+    # Check the size of a function pointer and of a data pointer.  If
+    # either of them is bigger than 4-bytes, assume our target has 64-bit
+    # addresses that are not supported by SREC, IHEX and TEKHEX.  We
+    # skip those tests then.
+    # If we error out below, we use the defaults (8 bytes) and skip
+    # the SREC, IHEX and TEKHEX tests just to be safe.
+    
+    set sizeof_function_ptr [get_sizeof "&main" 8]
+    set sizeof_data_ptr [get_sizeof "&intarray" 8]
+
+    if {${sizeof_function_ptr} > 4 || ${sizeof_data_ptr} > 4} then {
+	set is64bitonly "yes"
+    }
+}
+
 # Clean up any stale output files from previous test runs
 
 remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex intstr2.srec intstr2.tekhex intarr3.srec"

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Pedro Alves]

On 07/03/2013 08:22 PM, Luis Machado wrote:
> On 07/03/2013 12:05 PM, Pedro Alves wrote:
>> On 07/02/2013 07:50 PM, Luis Machado wrote:
>>> -
>>> -if {[istarget "spu*-*-*"]} then {
>>> -    # The internal address format used for the combined Cell/B.E.
>>> -    # debugger requires 64-bit.
>>> -    set is64bitonly "yes"
>>> -}
>>> -
>>
>> I'm not sure this equates to sizeof pointer == 64-bit.
>> This bit may need to be retained.  [Adding Ulrich].
> 
> Fair enough. Ulrich, let me know if the pointer check in the attached 
> patch doesn't make sense for Cell BE.
> 
>>
>>> +
>>> +    set sizeof_function_ptr [get_sizeof "void (*)(void)" 8]
>>> +    set sizeof_data_ptr [get_sizeof "void *" 8]
>>> +    if {${sizeof_function_ptr} != 4 && ${sizeof_data_ptr} != 4} then {
>>> +	set is64bitonly "yes"
>>> +    }
>>> +}
>>> +
>>
>> srec (etc.) is most used in small embedded targets (e.g., those
>> that include dsrec.o in the configure.tgt), consequently
>> that's where the test is most useful.  Such targets
>> are the most likely to have 16-bit pointers (< 4 bytes).
>> E.g., h8300, etc.  Looks like this ends up causing the tests to
>> be skipped there too.  IOW, a better check would be:
>>
>>     if {${sizeof_function_ptr} > 4 || ${sizeof_data_ptr} > 4} then {
>>
> 
> Ah, yes. This check is indeed better. Follows an updated patch that does 
> this.
> 
>> But, this change also means we have reduced routine-checking,
>> as most people test on x86_64.  I think we can do better.  The test
>> works fine on e.g., x86_64, because programs get linked to low (< 32-bit)
>> addresses by default.  That's the point of:
>>
>> if [istarget "alpha*-*-*"] then {
>>      # SREC etc cannot handle 64-bit addresses.  Force the test
>>      # program into the low 31 bits of the address space.
>>      lappend options "additional_flags=-Wl,-taso"
>> }
>>
>> (For MIPS N64, if you wanted, I guess you could do similarly
>>   to Alpha, and rebuild with:
>>
>>    lappend options "ldflags=-Wl,-Tdata=0x600000"
>>
>>   to force use of low addresses.)
>>
>> IOW, instead of checking for ABI pointer sizes, I think it'd
>> be better to test for the actual address size of one the
>> variables dumped.  That is, check that &intarray is < 32-bit.
>>
> 
> If lack of coverage for x86_64 running things on low addresses is a 
> problem, we can add an exception for x86_64, what do you think? Adding 
> these exceptions usually polute the testcases though.

But do you see a problem with checking whether "&intarray is < 32-bit" instead?

-- 
Pedro Alves

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]

On 07/03/2013 04:25 PM, Pedro Alves wrote:
> On 07/03/2013 08:22 PM, Luis Machado wrote:
>> On 07/03/2013 12:05 PM, Pedro Alves wrote:
>>> On 07/02/2013 07:50 PM, Luis Machado wrote:
>>>> -
>>>> -if {[istarget "spu*-*-*"]} then {
>>>> -    # The internal address format used for the combined Cell/B.E.
>>>> -    # debugger requires 64-bit.
>>>> -    set is64bitonly "yes"
>>>> -}
>>>> -
>>>
>>> I'm not sure this equates to sizeof pointer == 64-bit.
>>> This bit may need to be retained.  [Adding Ulrich].
>>
>> Fair enough. Ulrich, let me know if the pointer check in the attached
>> patch doesn't make sense for Cell BE.
>>
>>>
>>>> +
>>>> +    set sizeof_function_ptr [get_sizeof "void (*)(void)" 8]
>>>> +    set sizeof_data_ptr [get_sizeof "void *" 8]
>>>> +    if {${sizeof_function_ptr} != 4 && ${sizeof_data_ptr} != 4} then {
>>>> +	set is64bitonly "yes"
>>>> +    }
>>>> +}
>>>> +
>>>
>>> srec (etc.) is most used in small embedded targets (e.g., those
>>> that include dsrec.o in the configure.tgt), consequently
>>> that's where the test is most useful.  Such targets
>>> are the most likely to have 16-bit pointers (< 4 bytes).
>>> E.g., h8300, etc.  Looks like this ends up causing the tests to
>>> be skipped there too.  IOW, a better check would be:
>>>
>>>      if {${sizeof_function_ptr} > 4 || ${sizeof_data_ptr} > 4} then {
>>>
>>
>> Ah, yes. This check is indeed better. Follows an updated patch that does
>> this.
>>
>>> But, this change also means we have reduced routine-checking,
>>> as most people test on x86_64.  I think we can do better.  The test
>>> works fine on e.g., x86_64, because programs get linked to low (< 32-bit)
>>> addresses by default.  That's the point of:
>>>
>>> if [istarget "alpha*-*-*"] then {
>>>       # SREC etc cannot handle 64-bit addresses.  Force the test
>>>       # program into the low 31 bits of the address space.
>>>       lappend options "additional_flags=-Wl,-taso"
>>> }
>>>
>>> (For MIPS N64, if you wanted, I guess you could do similarly
>>>    to Alpha, and rebuild with:
>>>
>>>     lappend options "ldflags=-Wl,-Tdata=0x600000"
>>>
>>>    to force use of low addresses.)
>>>
>>> IOW, instead of checking for ABI pointer sizes, I think it'd
>>> be better to test for the actual address size of one the
>>> variables dumped.  That is, check that &intarray is < 32-bit.
>>>
>>
>> If lack of coverage for x86_64 running things on low addresses is a
>> problem, we can add an exception for x86_64, what do you think? Adding
>> these exceptions usually polute the testcases though.
>
> But do you see a problem with checking whether "&intarray is < 32-bit" instead?
>

Not really. Like so?

Luis

[-- Attachment #2: dump.diff --]
[-- Type: text/x-patch, Size: 1643 bytes --]

2013-07-03  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/dump.exp: Remove arch-specific tests and do a
	generic data address check to set is64bitonly correctly.

Index: gdb/testsuite/gdb.base/dump.exp
===================================================================
--- gdb/testsuite/gdb.base/dump.exp	(revision 415997)
+++ gdb/testsuite/gdb.base/dump.exp	(working copy)
@@ -32,16 +32,6 @@
     lappend options "additional_flags=-Wl,-taso"
 }
 
-if {[istarget "ia64*-*-*"] || [istarget "hppa64-*-*"]} then {
-    set is64bitonly "yes"
-}
-
-if {[istarget "spu*-*-*"]} then {
-    # The internal address format used for the combined Cell/B.E.
-    # debugger requires 64-bit.
-    set is64bitonly "yes"
-}
-
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ${options}] != "" } {
      untested dump.exp
      return -1
@@ -58,6 +48,15 @@
 
 gdb_load ${binfile}
 
+# Check the address of a variable.  If it is bigger than 32-bit,
+# assume our target has 64-bit addresses that are not supported by SREC,
+# IHEX and TEKHEX.  We skip those tests then.
+set max_32bit_address "0xffffffff"
+set data_address [get_hexadecimal_valueof "&intarray" 0x100000000]
+if {${data_address} > ${max_32bit_address}} then {
+    set is64bitonly "yes"
+}
+
 # Clean up any stale output files from previous test runs
 
 remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex intstr2.srec intstr2.tekhex intarr3.srec"

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Pedro Alves]

On 07/03/2013 09:19 PM, Luis Machado wrote:
> On 07/03/2013 04:25 PM, Pedro Alves wrote:

>> But do you see a problem with checking whether "&intarray is < 32-bit" instead?
>>
> 
> Not really. Like so?

Yep, exactly like that.

Thanks,
-- 
Pedro Alves

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On 07/04/2013 05:10 AM, Pedro Alves wrote:
> On 07/03/2013 09:19 PM, Luis Machado wrote:
>> On 07/03/2013 04:25 PM, Pedro Alves wrote:
>
>>> But do you see a problem with checking whether "&intarray is < 32-bit" instead?
>>>
>>
>> Not really. Like so?
>
> Yep, exactly like that.
>
> Thanks,
>

Thanks. Here is what i checked in. I restored the SPU exception.

Luis

[-- Attachment #2: dump_exp.diff --]
[-- Type: text/x-patch, Size: 1560 bytes --]

2013-07-05  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/dump.exp: Remove arch-specific tests and do a
	generic data address check to set is64bitonly correctly.

diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
index ea93b89..eb9be90 100644
--- a/gdb/testsuite/gdb.base/dump.exp
+++ b/gdb/testsuite/gdb.base/dump.exp
@@ -30,10 +30,6 @@ if [istarget "alpha*-*-*"] then {
     lappend options "additional_flags=-Wl,-taso"
 }
 
-if {[istarget "ia64*-*-*"] || [istarget "hppa64-*-*"]} then {
-    set is64bitonly "yes"
-}
-
 if {[istarget "spu*-*-*"]} then {
     # The internal address format used for the combined Cell/B.E.
     # debugger requires 64-bit.
@@ -56,6 +52,15 @@ gdb_test "dump mem /dev/null 0x10 0x20" "Cannot access memory at address 0x10" \
 
 gdb_load ${binfile}
 
+# Check the address of a variable.  If it is bigger than 32-bit,
+# assume our target has 64-bit addresses that are not supported by SREC,
+# IHEX and TEKHEX.  We skip those tests then.
+set max_32bit_address "0xffffffff"
+set data_address [get_hexadecimal_valueof "&intarray" 0x100000000]
+if {${data_address} > ${max_32bit_address}} then {
+    set is64bitonly "yes"
+}
+
 # Clean up any stale output files from previous test runs
 
 remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex intstr2.srec intstr2.tekhex intarr3.srec"

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Maciej W. Rozycki]

On Wed, 3 Jul 2013, Luis Machado wrote:

> > if [istarget "alpha*-*-*"] then {
> >      # SREC etc cannot handle 64-bit addresses.  Force the test
> >      # program into the low 31 bits of the address space.
> >      lappend options "additional_flags=-Wl,-taso"
> > }
> > 
> > (For MIPS N64, if you wanted, I guess you could do similarly
> >   to Alpha, and rebuild with:
> > 
> >    lappend options "ldflags=-Wl,-Tdata=0x600000"
> > 
> >   to force use of low addresses.)
[...]
> 
> As for MIPS, attempting to force the use of low addresses, just like alpha,
> seems to do more than what the tools expect at the moment, and i get a SIGSEGV
> in the dynamic loader.

 Hmm, while (unlike Alpha's -taso option) there is no way to force an 
entire n64 MIPS process into the 31-bit address space, the dynamic 
executable itself should work just fine mapped low.  However the default 
linker script relies on the start address of the text (0x120000000, unless 
overridden) rather than data segment to get things right and moving the 
linker's output address pointer backwards in the middle of the binary 
being linker may yield strange results.  Can you try (or have you tried) 
-Ttext=... instead?

  Maciej

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/03/2013 05:35 PM, Maciej W. Rozycki wrote:
> On Wed, 3 Jul 2013, Luis Machado wrote:
>
>>> if [istarget "alpha*-*-*"] then {
>>>       # SREC etc cannot handle 64-bit addresses.  Force the test
>>>       # program into the low 31 bits of the address space.
>>>       lappend options "additional_flags=-Wl,-taso"
>>> }
>>>
>>> (For MIPS N64, if you wanted, I guess you could do similarly
>>>    to Alpha, and rebuild with:
>>>
>>>     lappend options "ldflags=-Wl,-Tdata=0x600000"
>>>
>>>    to force use of low addresses.)
> [...]
>>
>> As for MIPS, attempting to force the use of low addresses, just like alpha,
>> seems to do more than what the tools expect at the moment, and i get a SIGSEGV
>> in the dynamic loader.
>
>   Hmm, while (unlike Alpha's -taso option) there is no way to force an
> entire n64 MIPS process into the 31-bit address space, the dynamic
> executable itself should work just fine mapped low.  However the default
> linker script relies on the start address of the text (0x120000000, unless
> overridden) rather than data segment to get things right and moving the
> linker's output address pointer backwards in the middle of the binary
> being linker may yield strange results.  Can you try (or have you tried)
> -Ttext=... instead?

I don't see a segfault with Ttext, but the text addresses are still 
based on 0x120000000.

Luis

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Maciej W. Rozycki]

On Wed, 3 Jul 2013, Luis Machado wrote:

> >   Hmm, while (unlike Alpha's -taso option) there is no way to force an
> > entire n64 MIPS process into the 31-bit address space, the dynamic
> > executable itself should work just fine mapped low.  However the default
> > linker script relies on the start address of the text (0x120000000, unless
> > overridden) rather than data segment to get things right and moving the
> > linker's output address pointer backwards in the middle of the binary
> > being linker may yield strange results.  Can you try (or have you tried)
> > -Ttext=... instead?
> 
> I don't see a segfault with Ttext, but the text addresses are still based on
> 0x120000000.

 Try -Ttext-segment=... instead, sorry about the confusion.

  Maciej

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/03/2013 06:08 PM, Maciej W. Rozycki wrote:
> On Wed, 3 Jul 2013, Luis Machado wrote:
>
>>>    Hmm, while (unlike Alpha's -taso option) there is no way to force an
>>> entire n64 MIPS process into the 31-bit address space, the dynamic
>>> executable itself should work just fine mapped low.  However the default
>>> linker script relies on the start address of the text (0x120000000, unless
>>> overridden) rather than data segment to get things right and moving the
>>> linker's output address pointer backwards in the middle of the binary
>>> being linker may yield strange results.  Can you try (or have you tried)
>>> -Ttext=... instead?
>>
>> I don't see a segfault with Ttext, but the text addresses are still based on
>> 0x120000000.
>
>   Try -Ttext-segment=... instead, sorry about the confusion.

That also doesn't seem to prevent thing from being linked at addresses > 
32-bits. Code is still at 0x120000000 apparently.

(gdb) break checkpoint1^M
Breakpoint 1 at 0x120000cdc: file gdb/testsuite/gdb.base/dump.c, line 19.

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Maciej W. Rozycki]

On Thu, 4 Jul 2013, Luis Machado wrote:

> >   Try -Ttext-segment=... instead, sorry about the confusion.
> 
> That also doesn't seem to prevent thing from being linked at addresses >
> 32-bits. Code is still at 0x120000000 apparently.
> 
> (gdb) break checkpoint1^M
> Breakpoint 1 at 0x120000cdc: file gdb/testsuite/gdb.base/dump.c, line 19.

 Works for me:

$ cat text.c
int main(void)
{
	return 0;
}
$ mips-linux-gnu-gcc -mabi=64 -O2 -Wl,-Ttext-segment=0x12340000 -o text text.c
$ mips-linux-gnu-readelf -l text

Elf file type is EXEC (Executable file)
Entry point 0x123407c0
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000012340040 0x0000000012340040
                 0x0000000000000188 0x0000000000000188  R E    8
  INTERP         0x0000000000000ab0 0x0000000012340ab0 0x0000000012340ab0
                 0x000000000000000f 0x000000000000000f  R      1
      [Requesting program interpreter: /lib64/ld.so.1]
  LOAD           0x0000000000000000 0x0000000012340000 0x0000000012340000
                 0x0000000000000ae4 0x0000000000000ae4  R E    10000
  LOAD           0x0000000000000ae8 0x0000000012350ae8 0x0000000012350ae8
                 0x00000000000000f0 0x0000000000000108  RW     10000
  DYNAMIC        0x0000000000000330 0x0000000012340330 0x0000000012340330
                 0x00000000000001b0 0x00000000000001b0  RWE    8
  NOTE           0x0000000000000ac4 0x0000000012340ac4 0x0000000012340ac4
                 0x0000000000000020 0x0000000000000020  R      4
  NULL           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000         8

 Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .MIPS.options .dynamic .hash .dynsym .dynstr .gnu.version .gnu.version_r .init .text .MIPS.stubs .fini .rodata .interp .eh_frame .note.ABI-tag
   03     .ctors .dtors .jcr .data .rld_map .got .sdata .bss
   04     .dynamic
   05     .note.ABI-tag
   06
$

  Maciej

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/04/2013 09:13 AM, Maciej W. Rozycki wrote:
> On Thu, 4 Jul 2013, Luis Machado wrote:
>
>>>    Try -Ttext-segment=... instead, sorry about the confusion.
>>
>> That also doesn't seem to prevent thing from being linked at addresses >
>> 32-bits. Code is still at 0x120000000 apparently.
>>
>> (gdb) break checkpoint1^M
>> Breakpoint 1 at 0x120000cdc: file gdb/testsuite/gdb.base/dump.c, line 19.
>
>   Works for me:
>
> $ cat text.c
> int main(void)
> {
> 	return 0;
> }
> $ mips-linux-gnu-gcc -mabi=64 -O2 -Wl,-Ttext-segment=0x12340000 -o text text.c
> $ mips-linux-gnu-readelf -l text
>
> Elf file type is EXEC (Executable file)
> Entry point 0x123407c0
> There are 7 program headers, starting at offset 64
>
> Program Headers:
>    Type           Offset             VirtAddr           PhysAddr
>                   FileSiz            MemSiz              Flags  Align
>    PHDR           0x0000000000000040 0x0000000012340040 0x0000000012340040
>                   0x0000000000000188 0x0000000000000188  R E    8
>    INTERP         0x0000000000000ab0 0x0000000012340ab0 0x0000000012340ab0
>                   0x000000000000000f 0x000000000000000f  R      1
>        [Requesting program interpreter: /lib64/ld.so.1]
>    LOAD           0x0000000000000000 0x0000000012340000 0x0000000012340000
>                   0x0000000000000ae4 0x0000000000000ae4  R E    10000
>    LOAD           0x0000000000000ae8 0x0000000012350ae8 0x0000000012350ae8
>                   0x00000000000000f0 0x0000000000000108  RW     10000
>    DYNAMIC        0x0000000000000330 0x0000000012340330 0x0000000012340330
>                   0x00000000000001b0 0x00000000000001b0  RWE    8
>    NOTE           0x0000000000000ac4 0x0000000012340ac4 0x0000000012340ac4
>                   0x0000000000000020 0x0000000000000020  R      4
>    NULL           0x0000000000000000 0x0000000000000000 0x0000000000000000
>                   0x0000000000000000 0x0000000000000000         8
>
>   Section to Segment mapping:
>    Segment Sections...
>     00
>     01     .interp
>     02     .MIPS.options .dynamic .hash .dynsym .dynstr .gnu.version .gnu.version_r .init .text .MIPS.stubs .fini .rodata .interp .eh_frame .note.ABI-tag
>     03     .ctors .dtors .jcr .data .rld_map .got .sdata .bss
>     04     .dynamic
>     05     .note.ABI-tag
>     06
> $
>
>    Maciej
>

Doh. Of course, i messed up the line that appends the linker option. 
With this change (done the right way) i see full passes for MIPS 64. 
Thanks for the info!

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Ulrich Weigand]

Luis Machado wrote:
> On 07/03/2013 12:05 PM, Pedro Alves wrote:
> > On 07/02/2013 07:50 PM, Luis Machado wrote:
> >> -
> >> -if {[istarget "spu*-*-*"]} then {
> >> -    # The internal address format used for the combined Cell/B.E.
> >> -    # debugger requires 64-bit.
> >> -    set is64bitonly "yes"
> >> -}
> >> -
> >
> > I'm not sure this equates to sizeof pointer == 64-bit.
> > This bit may need to be retained.  [Adding Ulrich].
> 
> Fair enough. Ulrich, let me know if the pointer check in the attached 
> patch doesn't make sense for Cell BE.

No, the pointer check is not enough.   The problem is that on Cell/B.E.,
GDB uses a (purely internal to GDB) address format to encode multiple
address spaces into a synthetic GDB address space.  See the long
comment in spu-tdep.h for details.

The "restore" command needs a full internal 64-bit address on Cell/B.E.
to uniquely identify the location where to restore memory.  Thus using
a data format that can only remember 32-bit addresses don't work:
http://sourceware.org/ml/gdb-patches/2008-09/msg00139.html

However, just testing the size of a pointer will return 32-bit on SPU,
because that's the ABI-defined pointer size (the 64 bits are purely
GDB internal).  So I think Cell/B.E. / SPU will need to remain a
special case here for now.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH, testsuite] Don't run SREC, IHEX and TEKHEX tests for MIPS N64.

[Luis Machado]

On 07/04/2013 10:22 AM, Ulrich Weigand wrote:
> Luis Machado wrote:
>> On 07/03/2013 12:05 PM, Pedro Alves wrote:
>>> On 07/02/2013 07:50 PM, Luis Machado wrote:
>>>> -
>>>> -if {[istarget "spu*-*-*"]} then {
>>>> -    # The internal address format used for the combined Cell/B.E.
>>>> -    # debugger requires 64-bit.
>>>> -    set is64bitonly "yes"
>>>> -}
>>>> -
>>>
>>> I'm not sure this equates to sizeof pointer == 64-bit.
>>> This bit may need to be retained.  [Adding Ulrich].
>>
>> Fair enough. Ulrich, let me know if the pointer check in the attached
>> patch doesn't make sense for Cell BE.
>
> No, the pointer check is not enough.   The problem is that on Cell/B.E.,
> GDB uses a (purely internal to GDB) address format to encode multiple
> address spaces into a synthetic GDB address space.  See the long
> comment in spu-tdep.h for details.
>
> The "restore" command needs a full internal 64-bit address on Cell/B.E.
> to uniquely identify the location where to restore memory.  Thus using
> a data format that can only remember 32-bit addresses don't work:
> http://sourceware.org/ml/gdb-patches/2008-09/msg00139.html
>
> However, just testing the size of a pointer will return 32-bit on SPU,
> because that's the ABI-defined pointer size (the 64 bits are purely
> GDB internal).  So I think Cell/B.E. / SPU will need to remain a
> special case here for now.
>
> Bye,
> Ulrich
>

I see. I'll put the spu exception back into the testcase then.

Thanks,
Luis