[1/2] gdb: testsuite: no need to setup_xfail "arm-*-coff" in a2-run.exp

Message ID 1637236761-7973-2-git-send-email-yangtiezhu@loongson.cn
State New
Headers show
Series
  • Do some changes in gdb/testsuite/gdb.base/a2-run.exp
Related show

Commit Message

Tiezhu Yang Nov. 18, 2021, 11:59 a.m.
After commit 2ac93be70641 ("Remove arm-aout and arm-coff support"),
there is no need to setup_xfail "arm-*-coff", just remove them in
a2-run.exp.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

---
 gdb/testsuite/gdb.base/a2-run.exp | 8 --------
 1 file changed, 8 deletions(-)

-- 
2.1.0

Comments

Tom Tromey Nov. 18, 2021, 3:26 p.m. | #1
>>>>> ">" == Tiezhu Yang <yangtiezhu@loongson.cn> writes:


>> After commit 2ac93be70641 ("Remove arm-aout and arm-coff support"),

>> there is no need to setup_xfail "arm-*-coff", just remove them in

>> a2-run.exp.


Interesting.  I did "git grep -i 'arm.*coff' -- gdb" and found some
other stuff we could maybe remove:

    gdb/arm-tdep.c:/* arm_coff_make_msymbol_special()

Presumably this function isn't needed.

I think the entire arm-wince-tdep.c file can be removed.

What do you think?

>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>

>> ---

>>  gdb/testsuite/gdb.base/a2-run.exp | 8 --------

>>  1 file changed, 8 deletions(-)


This is ok.  Thank you.

Tom
Mike Frysinger via Gdb-patches Nov. 18, 2021, 3:35 p.m. | #2
On 11/18/21 12:26 PM, Tom Tromey wrote:
>>>>>> ">" == Tiezhu Yang <yangtiezhu@loongson.cn> writes:

> 

>>> After commit 2ac93be70641 ("Remove arm-aout and arm-coff support"),

>>> there is no need to setup_xfail "arm-*-coff", just remove them in

>>> a2-run.exp.

> 

> Interesting.  I did "git grep -i 'arm.*coff' -- gdb" and found some

> other stuff we could maybe remove:

> 

>      gdb/arm-tdep.c:/* arm_coff_make_msymbol_special()

> 

> Presumably this function isn't needed.

> 

> I think the entire arm-wince-tdep.c file can be removed.

> 

> What do you think?

> 


I think so too. Is wince in the deprecation list (if we even have one)? 
I don't recall if we have a policy for deprecating things and how long 
to wait for it.
Tom Tromey Nov. 18, 2021, 3:44 p.m. | #3
>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:


Luis> I think so too. Is wince in the deprecation list (if we even have
Luis> one)? I don't recall if we have a policy for deprecating things and
Luis> how long to wait for it.

I don't recall offhand, either, but if the BFD support is gone, then
presumably this code already can't work.

Tom
Mike Frysinger via Gdb-patches Nov. 18, 2021, 3:46 p.m. | #4
On 11/18/21 12:44 PM, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

> 

> Luis> I think so too. Is wince in the deprecation list (if we even have

> Luis> one)? I don't recall if we have a policy for deprecating things and

> Luis> how long to wait for it.

> 

> I don't recall offhand, either, but if the BFD support is gone, then

> presumably this code already can't work.


Indeed. I'm thinking of doing a cleanup for this so we can get rid of 
dead code.
Tiezhu Yang Nov. 19, 2021, 1:38 a.m. | #5
On 11/18/2021 11:46 PM, Luis Machado wrote:
> On 11/18/21 12:44 PM, Tom Tromey wrote:

>>>>>>> "Luis" == Luis Machado via Gdb-patches 

>>>>>>> <gdb-patches@sourceware.org> writes:

>>

>> Luis> I think so too. Is wince in the deprecation list (if we even have

>> Luis> one)? I don't recall if we have a policy for deprecating things 

>> and

>> Luis> how long to wait for it.

>>

>> I don't recall offhand, either, but if the BFD support is gone, then

>> presumably this code already can't work.

>

> Indeed. I'm thinking of doing a cleanup for this so we can get rid of 

> dead code.


Hi Tom and Luis,
Thank you very much for your discussions and suggestions.
If you are OK, should I update this patch or send a new patch to do the
following changes:
(1) remove the related code about arm_coff_make_msymbol_special() in 
gdb/arm-tdep.c
diff --git a/gdb/arm-tdep.cb/gdb/arm-tdep.c
index 7495434..3664045100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8678,8+8678,7@@ coff_sym_is_thumb (int val)
|| val == C_THUMBLABEL);
}
-/* arm_coff_make_msymbol_special()
- arm_elf_make_msymbol_special()
+/* arm_elf_make_msymbol_special()
These functions test whether the COFF or ELF symbol corresponds to
an address in thumb code, and set a "special"bit in a minimal
@@ -8696,13+8695,6@@ arm_elf_make_msymbol_special(asymbol *sym, struct 
minimal_symbol *msym)
}
static void
-arm_coff_make_msymbol_special(int val, struct minimal_symbol *msym)
-{
- if (coff_sym_is_thumb (val))
- MSYMBOL_SET_SPECIAL (msym);
-}
-
-static void
arm_record_special_symbol (struct gdbarch *gdbarch, struct objfile *objfile,
asymbol *sym)
{
@@ -9582,8+9574,6@@ arm_gdbarch_init (struct gdbarch_info info, struct 
gdbarch_list *arches)
/* Minsymbol frobbing. */
set_gdbarch_elf_make_msymbol_special (gdbarch, 
arm_elf_make_msymbol_special);
- set_gdbarch_coff_make_msymbol_special (gdbarch,
- arm_coff_make_msymbol_special);
set_gdbarch_record_special_symbol (gdbarch, arm_record_special_symbol);
/* Thumb-2IT block support. */
(2) remove gdb/arm-wince-tdep.c
Mike Frysinger via Gdb-patches Nov. 19, 2021, 11:41 a.m. | #6
On 11/18/21 10:38 PM, Tiezhu Yang wrote:
> On 11/18/2021 11:46 PM, Luis Machado wrote:

>> On 11/18/21 12:44 PM, Tom Tromey wrote:

>>>>>>>> "Luis" == Luis Machado via Gdb-patches 

>>>>>>>> <gdb-patches@sourceware.org> writes:

>>>

>>> Luis> I think so too. Is wince in the deprecation list (if we even have

>>> Luis> one)? I don't recall if we have a policy for deprecating things 

>>> and

>>> Luis> how long to wait for it.

>>>

>>> I don't recall offhand, either, but if the BFD support is gone, then

>>> presumably this code already can't work.

>>

>> Indeed. I'm thinking of doing a cleanup for this so we can get rid of 

>> dead code.

> 

> Hi Tom and Luis,

> Thank you very much for your discussions and suggestions.

> If you are OK, should I update this patch or send a new patch to do the

> following changes:

> (1) remove the related code about arm_coff_make_msymbol_special() in 

> gdb/arm-tdep.c


There may be more to remove after that one. Let me check if this is 
really not being used first.
Mike Frysinger via Gdb-patches Nov. 19, 2021, 1:45 p.m. | #7
On 11/18/21 12:44 PM, Tom Tromey wrote:
>>>>>> "Luis" == Luis Machado via Gdb-patches <gdb-patches@sourceware.org> writes:

> 

> Luis> I think so too. Is wince in the deprecation list (if we even have

> Luis> one)? I don't recall if we have a policy for deprecating things and

> Luis> how long to wait for it.

> 

> I don't recall offhand, either, but if the BFD support is gone, then

> presumably this code already can't work.


I did some research and Windows Embedded Compact 2013 has extended 
support until October 10th, 2023. I still see some references of 
regression testing with arm-wince-pe. So presumably BFD support is still 
there.

Though I think this is fairly untested on GDB's side, and we removed the 
gdbserver support.
Tom Tromey Nov. 19, 2021, 8:39 p.m. | #8
Luis> I did some research and Windows Embedded Compact 2013 has extended
Luis> support until October 10th, 2023. I still see some references of 
Luis> regression testing with arm-wince-pe. So presumably BFD support is
Luis> still there.

Yeah, but the original patch pointed out that commit 2ac93be70641
removed arm-aout and arm-coff from BFD.  Without arm-coff in BFD, it
seems to me that this line in arm-wince-tdep.c will never cause the
sniffer to run:

  gdbarch_register_osabi_sniffer (bfd_arch_arm, bfd_target_coff_flavour,
				  arm_wince_osabi_sniffer);

Then, since that's the only thing that can return GDB_OSABI_WINCE, I
think the rest of the file must also be unusable.

thanks,
Tom

Patch

diff --git a/gdb/testsuite/gdb.base/a2-run.exp b/gdb/testsuite/gdb.base/a2-run.exp
index cf35f67..2042c26 100644
--- a/gdb/testsuite/gdb.base/a2-run.exp
+++ b/gdb/testsuite/gdb.base/a2-run.exp
@@ -130,21 +130,16 @@  if [target_info exists noargs] then {
 }
 
 # Now run with some arguments
-setup_xfail "arm-*-coff"
 gdb_run_cmd 5
 gdb_test_stdio "" "120" "" "run \"$testfile\" with arg"
 
 # Run again with same arguments.
 gdb_run_cmd 5
-
-setup_xfail "arm-*-coff"
 gdb_test_stdio "" "120" "" "run \"$testfile\" again with same args"
 
 # Use "set args" command to specify no arguments as default and run again.
 gdb_test_no_output "set args"
-
 gdb_run_cmd
-
 gdb_test_stdio "" "usage:  factorial <number>" "" "run after setting args to nil"
 
 # The remaining tests pass inferior arguments through GDB, so doesn't
@@ -158,10 +153,7 @@  if [use_gdb_stub] {
 
 # Use "set args" command to specify an argument and run again.
 gdb_test_no_output "set args 6"
-
 gdb_run_cmd
-
-setup_xfail "arm-*-coff"
 gdb_test_stdio "" "720" "" "run \"$testfile\" again after setting args"
 
 # GOAL: Test that shell is being used with "run".  For remote debugging