Skip gdb.base/func-ptrs.exp in thumb mode

Message ID 1465812702-22061-1-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi June 13, 2016, 10:11 a.m. UTC
  This test case was added
https://sourceware.org/ml/gdb-patches/2014-10/msg00088.html to teach
GDB to treat ISA bit on MIPS.  I thought we can do the same for ARM
thumb mode, but I failed to do so due to the difference of mips16
and thumb.

As Maciej described in
https://sourceware.org/ml/gdb-patches/2012-05/msg00515.html
"Any instruction executed at an even address is interpreted as a
standard MIPS instruction (the address still has to have its bit
 #1 clear), any instruction executed at an odd address is
interpreted as a MIPS16 instruction.", it is natural to have
instructions at odd addresses, see the disassembly in current GDB
for mips16 code,

(gdb) disassemble incr
Dump of assembler code for function incr:
   0x0040055d <+0>:	save	a0,32,ra,s1
   0x00400561 <+4>:	addiu	s1,sp,16
   0x00400563 <+6>:	jal	0x400551 <sentinel>

However, arm does the trick by using the right branch instruction
(bx and blx) together with setting the LSB in the target address.
The instructions are still at even addresses.

Due to this difference on address, we have troubles mapping stop
address to line table on thumb mode, because we've set the LSB in
address in line table, as what we did for mips, but the stop address
reported by the hardware is still an even one, so we'll get a fail,

Breakpoint 3, 0x00008604 in incr (i=-1) at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/func-ptrs.c:37^M
37      {^M
(gdb) FAIL: gdb.base/func-ptrs.exp: continue to incr, first time

the hex 0x00008604 is unexpected by the test, because it is not the
starting address of line table entry, so the hex is printed,

3          37 0x00000000000085fd
4          38 0x0000000000008605
5          39 0x0000000000008609

In fact, my prototype GDB works well except on this address-line
mapping.  I didn't try to tweak target to report the address with
LSB set, because it makes no sense on ARM thumb mode.

gdb/testsuite:

2016-06-13  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/func-ptrs.c [__thumb__]: Set supported to 0.
	[!__thumb__]: Set supported to 1.
	* gdb.base/func-ptrs.exp: Return early if variable supported
	value is 0.
---
 gdb/testsuite/gdb.base/func-ptrs.c   | 11 +++++++++++
 gdb/testsuite/gdb.base/func-ptrs.exp |  4 ++++
 2 files changed, 15 insertions(+)
  

Comments

Maciej W. Rozycki June 22, 2016, 7:04 p.m. UTC | #1
On Mon, 13 Jun 2016, Yao Qi wrote:

> This test case was added
> https://sourceware.org/ml/gdb-patches/2014-10/msg00088.html to teach
> GDB to treat ISA bit on MIPS.  I thought we can do the same for ARM
> thumb mode, but I failed to do so due to the difference of mips16
> and thumb.

 FWIW it looks like an ABI deficiency to me then, perhaps just in the 
debug support part.  You ought to be able to make the same operations in 
GDB (e.g. function pointer assignments) as you do at the source level of 
your programming language (e.g. C), and yield the same results.  Of 
course it doesn't have to be an ISA bit that guarantees consistency 
though.

> As Maciej described in
> https://sourceware.org/ml/gdb-patches/2012-05/msg00515.html
> "Any instruction executed at an even address is interpreted as a
> standard MIPS instruction (the address still has to have its bit
>  #1 clear), any instruction executed at an odd address is
> interpreted as a MIPS16 instruction.", it is natural to have
> instructions at odd addresses, see the disassembly in current GDB
> for mips16 code,
> 
> (gdb) disassemble incr
> Dump of assembler code for function incr:
>    0x0040055d <+0>:	save	a0,32,ra,s1
>    0x00400561 <+4>:	addiu	s1,sp,16
>    0x00400563 <+6>:	jal	0x400551 <sentinel>
> 
> However, arm does the trick by using the right branch instruction
> (bx and blx) together with setting the LSB in the target address.

 So how does this work for indirect calls, such as ones made via function 
pointers, with the ARM target?  With dynamic reassignment such a pointer 
can change at the run time and with mixed code it can point to regular ARM 
code sometimes and at other times it can point to Thumb code, still within 
the same execution of a program.  Can't GDB reproduce the same actions?  
What do GCC/GAS/LD do with function pointer assignments that GDB can't?

> The instructions are still at even addresses.
> 
> Due to this difference on address, we have troubles mapping stop
> address to line table on thumb mode, because we've set the LSB in
> address in line table, as what we did for mips, but the stop address
> reported by the hardware is still an even one, so we'll get a fail,
> 
> Breakpoint 3, 0x00008604 in incr (i=-1) at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/func-ptrs.c:37^M
> 37      {^M
> (gdb) FAIL: gdb.base/func-ptrs.exp: continue to incr, first time
> 
> the hex 0x00008604 is unexpected by the test, because it is not the
> starting address of line table entry, so the hex is printed,
> 
> 3          37 0x00000000000085fd
> 4          38 0x0000000000008605
> 5          39 0x0000000000008609
> 
> In fact, my prototype GDB works well except on this address-line
> mapping.  I didn't try to tweak target to report the address with
> LSB set, because it makes no sense on ARM thumb mode.

 Hmm, so how does breakpoint matching work in the absence of function 
pointer reassignment?

> diff --git a/gdb/testsuite/gdb.base/func-ptrs.exp
> b/gdb/testsuite/gdb.base/func-ptrs.exp
> index a3b55a8748..b3ea1ce 100644
> --- a/gdb/testsuite/gdb.base/func-ptrs.exp
> +++ b/gdb/testsuite/gdb.base/func-ptrs.exp
> @@ -24,6 +24,10 @@ if { ![runto_main] } {
>      return -1
>  }
>  
> +if { 0 == [get_integer_valueof "supported" "0"] } {
> +    unsupported "function pointer assignment is not supported"
> +    return 1
> +}
>  
>  # First set our breakpoints.
>  

 Not `kfail ...'?

  Maciej
  
Yao Qi June 29, 2016, 8:39 a.m. UTC | #2
On Wed, Jun 22, 2016 at 8:04 PM, Maciej W. Rozycki <macro@imgtec.com> wrote:
>
>> As Maciej described in
>> https://sourceware.org/ml/gdb-patches/2012-05/msg00515.html
>> "Any instruction executed at an even address is interpreted as a
>> standard MIPS instruction (the address still has to have its bit
>>  #1 clear), any instruction executed at an odd address is
>> interpreted as a MIPS16 instruction.", it is natural to have
>> instructions at odd addresses, see the disassembly in current GDB
>> for mips16 code,
>>
>> (gdb) disassemble incr
>> Dump of assembler code for function incr:
>>    0x0040055d <+0>:   save    a0,32,ra,s1
>>    0x00400561 <+4>:   addiu   s1,sp,16
>>    0x00400563 <+6>:   jal     0x400551 <sentinel>
>>
>> However, arm does the trick by using the right branch instruction
>> (bx and blx) together with setting the LSB in the target address.
>
>  So how does this work for indirect calls, such as ones made via function
> pointers, with the ARM target?  With dynamic reassignment such a pointer
> can change at the run time and with mixed code it can point to regular ARM
> code sometimes and at other times it can point to Thumb code, still within
> the same execution of a program.  Can't GDB reproduce the same actions?
> What do GCC/GAS/LD do with function pointer assignments that GDB can't?

On ARM target, instruction "blx rm" is generated for indirect call, and "rm" has
the target address, plus ISA bit if needed.  If the target function is ARM code,
"rm" has the target function address, otherwise, it has the thumb
target function
address with LSB set.  I want to make it clear that, although thumb function
symbol value has LSB set, which only indicates this is a thumb function, the
function address is still 2-byte aligned.  All these ISA bit set/clear
is managed
by LD.

However, LD doesn't need to handle the address reported in the run time.
When we set a breakpoint on a certain address in thumb code, the address
must be 2-byte aligned, and we get troubles on matching the reported address
with adjusted symbols (with LSB set).  I suspect the reported address on MIPS
has ISB bit set.

>
>> The instructions are still at even addresses.
>>
>> Due to this difference on address, we have troubles mapping stop
>> address to line table on thumb mode, because we've set the LSB in
>> address in line table, as what we did for mips, but the stop address
>> reported by the hardware is still an even one, so we'll get a fail,
>>
>> Breakpoint 3, 0x00008604 in incr (i=-1) at /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/func-ptrs.c:37^M
>> 37      {^M
>> (gdb) FAIL: gdb.base/func-ptrs.exp: continue to incr, first time
>>
>> the hex 0x00008604 is unexpected by the test, because it is not the
>> starting address of line table entry, so the hex is printed,
>>
>> 3          37 0x00000000000085fd
>> 4          38 0x0000000000008605
>> 5          39 0x0000000000008609
>>
>> In fact, my prototype GDB works well except on this address-line
>> mapping.  I didn't try to tweak target to report the address with
>> LSB set, because it makes no sense on ARM thumb mode.
>
>  Hmm, so how does breakpoint matching work in the absence of function
> pointer reassignment?
>

In my prototype GDB, it doesn't work either, because breakpoint is set at 2-byte
aligned address, but the LSB of address in line table is set.

>> diff --git a/gdb/testsuite/gdb.base/func-ptrs.exp
>> b/gdb/testsuite/gdb.base/func-ptrs.exp
>> index a3b55a8748..b3ea1ce 100644
>> --- a/gdb/testsuite/gdb.base/func-ptrs.exp
>> +++ b/gdb/testsuite/gdb.base/func-ptrs.exp
>> @@ -24,6 +24,10 @@ if { ![runto_main] } {
>>      return -1
>>  }
>>
>> +if { 0 == [get_integer_valueof "supported" "0"] } {
>> +    unsupported "function pointer assignment is not supported"
>> +    return 1
>> +}
>>
>>  # First set our breakpoints.
>>
>
>  Not `kfail ...'?
>

Hmm, either is fine to me, so I don't mind changing it to kfail.
  
Pedro Alves June 29, 2016, 11:55 a.m. UTC | #3
On 06/29/2016 09:39 AM, Yao Qi wrote:
> On Wed, Jun 22, 2016 at 8:04 PM, Maciej W. Rozycki <macro@imgtec.com> wrote:
>>

>>  So how does this work for indirect calls, such as ones made via function
>> pointers, with the ARM target?  With dynamic reassignment such a pointer
>> can change at the run time and with mixed code it can point to regular ARM
>> code sometimes and at other times it can point to Thumb code, still within
>> the same execution of a program.  Can't GDB reproduce the same actions?
>> What do GCC/GAS/LD do with function pointer assignments that GDB can't?
> 
> On ARM target, instruction "blx rm" is generated for indirect call, and "rm" has
> the target address, plus ISA bit if needed.  If the target function is ARM code,
> "rm" has the target function address, otherwise, it has the thumb
> target function
> address with LSB set.  I want to make it clear that, although thumb function
> symbol value has LSB set, which only indicates this is a thumb function, the
> function address is still 2-byte aligned.  All these ISA bit set/clear
> is managed
> by LD.

I think the question is what happens at run time, when a function
pointer is set to a thumb function.  Say, with something like this:

typedef void (func_ptr) (void);

void arm_function (void);
void thumb_function (void); // add whatever __attribute__ necessary
                            // for thumb.

func_ptr *ptr;

void foo (void)
{
   ptr = thumb_function;
   ptr ();  // stop here
}

Does "ptr" hold an address with the LSB set at run time, somehow?
I assume it does.

Then, doesn't this in gdb:

 (gdb) print ptr = thumb_function

end up with the same value (LSB set)?  If not, seems like that's a bug.


Note that from the original commit log, one can't tell what
exactly goes wrong in the test that requires skipping it.  It talks
in terms of Maciej's patch, and how some prototype based on that
behaves, and not in terms of what is wrong with the current support, or
if there's something the test is doing that is fundamentally impossible
to do on arm.

Thanks,
Pedro Alves
  
Yao Qi June 29, 2016, 4:25 p.m. UTC | #4
On Wed, Jun 29, 2016 at 12:55 PM, Pedro Alves <palves@redhat.com> wrote:
>
> I think the question is what happens at run time, when a function
> pointer is set to a thumb function.  Say, with something like this:
>
> typedef void (func_ptr) (void);
>
> void arm_function (void);
> void thumb_function (void); // add whatever __attribute__ necessary
>                             // for thumb.
>
> func_ptr *ptr;
>
> void foo (void)
> {
>    ptr = thumb_function;
>    ptr ();  // stop here
> }
>
> Does "ptr" hold an address with the LSB set at run time, somehow?
> I assume it does.

Yes, it does.

>
> Then, doesn't this in gdb:
>
>  (gdb) print ptr = thumb_function
>
> end up with the same value (LSB set)?  If not, seems like that's a bug.
>

ptr doesn't have LSB set, because the value of symbol "thumb_function"
doesn't have LSB set in GDB.

> Note that from the original commit log, one can't tell what
> exactly goes wrong in the test that requires skipping it.  It talks
> in terms of Maciej's patch, and how some prototype based on that
> behaves, and not in terms of what is wrong with the current support, or
> if there's something the test is doing that is fundamentally impossible
> to do on arm.

The test has nothing wrong, but current approach, added by Maciej, can't
be extended for ARM thumb mode, as a result of my investigation and
prototype.  In current approach, we propagate ISA bit of address into symbols
and line tables, and that works for MIPS.  In ARM, the LSB of address
only indicates that instruction set will be changed to thumb, and
function is still
2-byte aligned.  For example, thumb_function's address is 0x00010510, and
it really starts from there.  We only set LSB of address, or set it to
0x00010511,
when it is the target address of branch with changing mode from arm to thumb.
Set LSB of address in other places make no sense to ARM.

I also find that gdb.base/func-ptrs.exp fails on ppc64, due to the function
descriptor.  GDB should assign function descriptor to the pointer rather than
the function address.  I am thinking maybe we can use a different approach
for arm and ppc64, that is, when we do function pointer assignment, use a
gdbarch method to adjust the result, on arm, if the result is the entry of a
thumb function, set its LSB; on ppc64, if the result is the entry of a function,
set it to the address of function descriptor.  However, I don't know where is
the best place (in expression evaluation?) to adjust the result of function
pointer assignment.
  
Pedro Alves June 29, 2016, 4:31 p.m. UTC | #5
On 06/29/2016 05:25 PM, Yao Qi wrote:
> On Wed, Jun 29, 2016 at 12:55 PM, Pedro Alves <palves@redhat.com> wrote:
>>
>> I think the question is what happens at run time, when a function
>> pointer is set to a thumb function.  Say, with something like this:
>>
>> typedef void (func_ptr) (void);
>>
>> void arm_function (void);
>> void thumb_function (void); // add whatever __attribute__ necessary
>>                             // for thumb.
>>
>> func_ptr *ptr;
>>
>> void foo (void)
>> {
>>    ptr = thumb_function;
>>    ptr ();  // stop here
>> }
>>
>> Does "ptr" hold an address with the LSB set at run time, somehow?
>> I assume it does.
> 
> Yes, it does.
> 
>>
>> Then, doesn't this in gdb:
>>
>>  (gdb) print ptr = thumb_function
>>
>> end up with the same value (LSB set)?  If not, seems like that's a bug.
>>
> 
> ptr doesn't have LSB set, because the value of symbol "thumb_function"
> doesn't have LSB set in GDB.
> 
>> Note that from the original commit log, one can't tell what
>> exactly goes wrong in the test that requires skipping it.  It talks
>> in terms of Maciej's patch, and how some prototype based on that
>> behaves, and not in terms of what is wrong with the current support, or
>> if there's something the test is doing that is fundamentally impossible
>> to do on arm.
> 
> The test has nothing wrong, but current approach, added by Maciej, can't
> be extended for ARM thumb mode, as a result of my investigation and
> prototype.  In current approach, we propagate ISA bit of address into symbols
> and line tables, and that works for MIPS.  In ARM, the LSB of address
> only indicates that instruction set will be changed to thumb, and
> function is still
> 2-byte aligned.  For example, thumb_function's address is 0x00010510, and
> it really starts from there.  We only set LSB of address, or set it to
> 0x00010511,
> when it is the target address of branch with changing mode from arm to thumb.
> Set LSB of address in other places make no sense to ARM.

Agreed.

> 
> I also find that gdb.base/func-ptrs.exp fails on ppc64, due to the function
> descriptor.  GDB should assign function descriptor to the pointer rather than
> the function address.  I am thinking maybe we can use a different approach
> for arm and ppc64, that is, when we do function pointer assignment, use a
> gdbarch method to adjust the result, on arm, if the result is the entry of a
> thumb function, set its LSB; on ppc64, if the result is the entry of a function,
> set it to the address of function descriptor.  However, I don't know where is
> the best place (in expression evaluation?) to adjust the result of function
> pointer assignment.

Off hand, I'd think somewhere around value_assign / value_cast / value_cast_pointers.
 
Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/testsuite/gdb.base/func-ptrs.c b/gdb/testsuite/gdb.base/func-ptrs.c
index 8a93c48..39efe2b 100644
--- a/gdb/testsuite/gdb.base/func-ptrs.c
+++ b/gdb/testsuite/gdb.base/func-ptrs.c
@@ -15,6 +15,17 @@ 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#if defined(__thumb__)
+/* In ARM, the last bit of the target address of branch indicates the
+   program mode is ARM or THUMB.  However, the last bit of the symbol
+   isn't set even in THUMB mode, so if we assign the function to a
+   function pointer in GDB, the last bit of the target address isn't
+   set.  */
+int supported = 0;
+#else
+int supported = 1;
+#endif
+
 void
 sentinel (void)
 {
diff --git a/gdb/testsuite/gdb.base/func-ptrs.exp b/gdb/testsuite/gdb.base/func-ptrs.exp
index a3b55a8748..b3ea1ce 100644
--- a/gdb/testsuite/gdb.base/func-ptrs.exp
+++ b/gdb/testsuite/gdb.base/func-ptrs.exp
@@ -24,6 +24,10 @@  if { ![runto_main] } {
     return -1
 }
 
+if { 0 == [get_integer_valueof "supported" "0"] } {
+    unsupported "function pointer assignment is not supported"
+    return 1
+}
 
 # First set our breakpoints.