public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
@ 2020-05-02  7:51 Tom de Vries
  2020-06-17 12:24 ` Gary Benson
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-05-02  7:51 UTC (permalink / raw)
  To: gdb-patches

Hi,

When running test-case gdb.base/psym-external-decl.exp with gcc-10, we have:
...
(gdb) print aaa^M
'aaa' has unknown type; cast it to its declared type^M
(gdb) FAIL: gdb.base/psym-external-decl.exp: print aaa
...

With an an earlier version, gcc still emits the debug info for the
declaration of aaa:
...
 <0><d2>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <d8>   DW_AT_name        : psym-external-decl.c
 <1><f4>: Abbrev Number: 2 (DW_TAG_variable)
    <f5>   DW_AT_name        : aaa
    <ff>   DW_AT_external    : 1
    <ff>   DW_AT_declaration : 1
...
but with gcc-10 that's no longer the case.

Fix the test-case by adding a use of aaa in psym-external-decl.c.

That still doesn't work for clang, so skip test in that case.

Tested with x86_64-linux, with gcc 7.5.0, gcc 10.0.0 and clang 5.0.2.

Also tested by reverting corresponding fix and ensuring test-case still
fails.

Committed to trunk.

Thanks,
- Tom

[gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang

gdb/testsuite/ChangeLog:

2020-05-02  Tom de Vries  <tdevries@suse.de>

	* gdb.base/psym-external-decl.c (main): Add use of variable aaa.

---
 gdb/testsuite/gdb.base/psym-external-decl.c   | 2 +-
 gdb/testsuite/gdb.base/psym-external-decl.exp | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/psym-external-decl.c b/gdb/testsuite/gdb.base/psym-external-decl.c
index 7a4b1077741..e2374327bde 100644
--- a/gdb/testsuite/gdb.base/psym-external-decl.c
+++ b/gdb/testsuite/gdb.base/psym-external-decl.c
@@ -20,6 +20,6 @@ extern int aaa;
 int
 main (void)
 {
-  return 0;
+  return aaa;
 }
 
diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
index bbcc2745755..d0388d5655e 100644
--- a/gdb/testsuite/gdb.base/psym-external-decl.exp
+++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
@@ -15,6 +15,11 @@
 
 standard_testfile .c psym-external-decl-2.c
 
+get_compiler_info
+if { [test_compiler_info "clang-*"] } {
+    return -1
+}
+
 set srcfiles [list $srcfile $srcfile2]
 
 if { [build_executable_from_specs \

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-05-02  7:51 [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang Tom de Vries
@ 2020-06-17 12:24 ` Gary Benson
  2020-06-17 13:56   ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Gary Benson @ 2020-06-17 12:24 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

Tom de Vries wrote:
> Fix the test-case [with gcc-10] by adding a use of aaa in psym-external-decl.c.
... 
> That still doesn't work for clang, so skip test in that case.
..
> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
> index bbcc2745755..d0388d5655e 100644
> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
> @@ -15,6 +15,11 @@
>  
>  standard_testfile .c psym-external-decl-2.c
>  
> +get_compiler_info
> +if { [test_compiler_info "clang-*"] } {
> +    return -1
> +}
> +
>  set srcfiles [list $srcfile $srcfile2]
>  
>  if { [build_executable_from_specs \

Tom, I'd like this testcase to not fail silently.  Is the
functionality under test something that isn't ever expected to work
with clang, or is this a test that should pass with clang (but it
currently doesn't, for whatever reason)?

If the former (it isn't ever expected to work), can this test please
call to unsupported/untested, so there's at least something in the
.sum file?  If it's the latter (the test should pass, but it doesn't),
can the skip-the-test block please be removed, so the test can fail?

Whichever it is, I'm happy to make the commit, or you can if you
like, I'm easy.

Thanks,
Gary


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-17 12:24 ` Gary Benson
@ 2020-06-17 13:56   ` Tom de Vries
  2020-06-18 16:10     ` Gary Benson
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-06-17 13:56 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 6/17/20 2:24 PM, Gary Benson wrote:
> Tom de Vries wrote:
>> Fix the test-case [with gcc-10] by adding a use of aaa in psym-external-decl.c.
> ... 
>> That still doesn't work for clang, so skip test in that case.
> ..
>> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
>> index bbcc2745755..d0388d5655e 100644
>> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
>> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
>> @@ -15,6 +15,11 @@
>>  
>>  standard_testfile .c psym-external-decl-2.c
>>  
>> +get_compiler_info
>> +if { [test_compiler_info "clang-*"] } {
>> +    return -1
>> +}
>> +
>>  set srcfiles [list $srcfile $srcfile2]
>>  
>>  if { [build_executable_from_specs \
> 
> Tom, I'd like this testcase to not fail silently.  Is the
> functionality under test something that isn't ever expected to work
> with clang, or is this a test that should pass with clang (but it
> currently doesn't, for whatever reason)?
> 

Hi Gary,

I'm not sure.  The test can pass with clang, provided it generates the
required debug info.  It currently doesn't.  Why that is the case, I
have no idea.

Thanks,
- Tom

> If the former (it isn't ever expected to work), can this test please
> call to unsupported/untested, so there's at least something in the
> .sum file?  If it's the latter (the test should pass, but it doesn't),
> can the skip-the-test block please be removed, so the test can fail?
> 
> Whichever it is, I'm happy to make the commit, or you can if you
> like, I'm easy.
> 
> Thanks,
> Gary
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-17 13:56   ` Tom de Vries
@ 2020-06-18 16:10     ` Gary Benson
  2020-06-18 16:27       ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Gary Benson @ 2020-06-18 16:10 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

Tom de Vries wrote:
> On 6/17/20 2:24 PM, Gary Benson wrote:
> > Tom de Vries wrote:
> > > Fix the test-case [with gcc-10] by adding a use of aaa in psym-external-decl.c.
> > ... 
> > > That still doesn't work for clang, so skip test in that case.
> > ..
> > > diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
> > > index bbcc2745755..d0388d5655e 100644
> > > --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
> > > +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
> > > @@ -15,6 +15,11 @@
> > >  
> > >  standard_testfile .c psym-external-decl-2.c
> > >  
> > > +get_compiler_info
> > > +if { [test_compiler_info "clang-*"] } {
> > > +    return -1
> > > +}
> > > +
> > >  set srcfiles [list $srcfile $srcfile2]
> > >  
> > >  if { [build_executable_from_specs \
> > 
> > Tom, I'd like this testcase to not fail silently.  Is the
> > functionality under test something that isn't ever expected to
> > work with clang, or is this a test that should pass with clang
> > (but it currently doesn't, for whatever reason)?
> 
> I'm not sure.  The test can pass with clang, provided it generates the
> required debug info.  It currently doesn't.  Why that is the case, I
> have no idea.

I think that means the test should work but it doesn't.  Would you
object if I push a patch removing the test-skipping logic?  It will
mean an extra FAIL when tested using clang

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-18 16:10     ` Gary Benson
@ 2020-06-18 16:27       ` Tom de Vries
  2020-06-19 14:00         ` Gary Benson
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-06-18 16:27 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 6/18/20 6:10 PM, Gary Benson wrote:
> Tom de Vries wrote:
>> On 6/17/20 2:24 PM, Gary Benson wrote:
>>> Tom de Vries wrote:
>>>> Fix the test-case [with gcc-10] by adding a use of aaa in psym-external-decl.c.
>>> ... 
>>>> That still doesn't work for clang, so skip test in that case.
>>> ..
>>>> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
>>>> index bbcc2745755..d0388d5655e 100644
>>>> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
>>>> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
>>>> @@ -15,6 +15,11 @@
>>>>  
>>>>  standard_testfile .c psym-external-decl-2.c
>>>>  
>>>> +get_compiler_info
>>>> +if { [test_compiler_info "clang-*"] } {
>>>> +    return -1
>>>> +}
>>>> +
>>>>  set srcfiles [list $srcfile $srcfile2]
>>>>  
>>>>  if { [build_executable_from_specs \
>>>
>>> Tom, I'd like this testcase to not fail silently.  Is the
>>> functionality under test something that isn't ever expected to
>>> work with clang, or is this a test that should pass with clang
>>> (but it currently doesn't, for whatever reason)?
>>
>> I'm not sure.  The test can pass with clang, provided it generates the
>> required debug info.  It currently doesn't.  Why that is the case, I
>> have no idea.
> 
> I think that means the test should work but it doesn't.  Would you
> object if I push a patch removing the test-skipping logic?  It will
> mean an extra FAIL when tested using clang
> 

Hi Gary,

I don't think having a fail for a compiler bug/missing-feature is a good
idea.

If this is due to a bug/missing-feature in clang, then we need to:
- xfail the test,
- file the PR in clang, and
- reference the PR at the xfail.

If this is a wontfix for clang, then we can mark it unsupported.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-18 16:27       ` Tom de Vries
@ 2020-06-19 14:00         ` Gary Benson
  2020-06-19 14:06           ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Gary Benson @ 2020-06-19 14:00 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

Tom de Vries wrote:
> On 6/18/20 6:10 PM, Gary Benson wrote:
> > Tom de Vries wrote:
> >> On 6/17/20 2:24 PM, Gary Benson wrote:
> >>> Tom de Vries wrote:
> >>>> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
> >>>> index bbcc2745755..d0388d5655e 100644
> >>>> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
> >>>> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
[nsip]
> >>> Tom, I'd like this testcase to not fail silently.  Is the
> >>> functionality under test something that isn't ever expected to
> >>> work with clang, or is this a test that should pass with clang
> >>> (but it currently doesn't, for whatever reason)?
> >>
> >> I'm not sure.  The test can pass with clang, provided it generates the
> >> required debug info.  It currently doesn't.  Why that is the case, I
> >> have no idea.
> > 
> > I think that means the test should work but it doesn't.  Would you
> > object if I push a patch removing the test-skipping logic?  It will
> > mean an extra FAIL when tested using clang
> 
> I don't think having a fail for a compiler bug/missing-feature is a
> good idea.
> 
> If this is due to a bug/missing-feature in clang, then we need to:
> - xfail the test,
> - file the PR in clang, and
> - reference the PR at the xfail.

Is this a bug/missing feature in clang though?
How sure are you GDB isn't at fault?

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-19 14:00         ` Gary Benson
@ 2020-06-19 14:06           ` Tom de Vries
  2020-06-26  9:37             ` Gary Benson
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-06-19 14:06 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 6/19/20 4:00 PM, Gary Benson wrote:
> Tom de Vries wrote:
>> On 6/18/20 6:10 PM, Gary Benson wrote:
>>> Tom de Vries wrote:
>>>> On 6/17/20 2:24 PM, Gary Benson wrote:
>>>>> Tom de Vries wrote:
>>>>>> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
>>>>>> index bbcc2745755..d0388d5655e 100644
>>>>>> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
>>>>>> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
> [nsip]
>>>>> Tom, I'd like this testcase to not fail silently.  Is the
>>>>> functionality under test something that isn't ever expected to
>>>>> work with clang, or is this a test that should pass with clang
>>>>> (but it currently doesn't, for whatever reason)?
>>>>
>>>> I'm not sure.  The test can pass with clang, provided it generates the
>>>> required debug info.  It currently doesn't.  Why that is the case, I
>>>> have no idea.
>>>
>>> I think that means the test should work but it doesn't.  Would you
>>> object if I push a patch removing the test-skipping logic?  It will
>>> mean an extra FAIL when tested using clang
>>
>> I don't think having a fail for a compiler bug/missing-feature is a
>> good idea.
>>
>> If this is due to a bug/missing-feature in clang, then we need to:
>> - xfail the test,
>> - file the PR in clang, and
>> - reference the PR at the xfail.
> 
> Is this a bug/missing feature in clang though?
> How sure are you GDB isn't at fault?

Clang emits less debug info than GCC.  Whether that's a bug, a missing
feature or an explicit unsupported feature in clang, I don't known.

I known that gdb isn't at fault.  It can't do anything without the
missing debug info.  The test was specifically written to use that debug
info.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-19 14:06           ` Tom de Vries
@ 2020-06-26  9:37             ` Gary Benson
  2020-06-28 10:50               ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Gary Benson @ 2020-06-26  9:37 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

Tom de Vries wrote:
> On 6/19/20 4:00 PM, Gary Benson wrote:
> > Tom de Vries wrote:
> > > On 6/18/20 6:10 PM, Gary Benson wrote:
> > > > Tom de Vries wrote:
> > > > > On 6/17/20 2:24 PM, Gary Benson wrote:
> > > > > > Tom, I'd like this testcase to not fail silently.  Is the
> > > > > > functionality under test something that isn't ever
> > > > > > expected to work with clang, or is this a test that should
> > > > > > pass with clang (but it currently doesn't, for whatever
> > > > > > reason)?
> > > > >
> > > > > I'm not sure.  The test can pass with clang, provided it
> > > > > generates the required debug info.  It currently doesn't.
> > > > > Why that is the case, I have no idea.
> > > >
> > > > I think that means the test should work but it doesn't.  Would
> > > > you object if I push a patch removing the test-skipping logic?
> > > > It will mean an extra FAIL when tested using clang
> > >
> > > I don't think having a fail for a compiler bug/missing-feature
> > > is a good idea.
> > >
> > > If this is due to a bug/missing-feature in clang, then we need to:
> > > - xfail the test,
> > > - file the PR in clang, and
> > > - reference the PR at the xfail.
> > 
> > Is this a bug/missing feature in clang though?
> > How sure are you GDB isn't at fault?
> 
> Clang emits less debug info than GCC.  Whether that's a bug, a
> missing feature or an explicit unsupported feature in clang, I
> don't known.
> 
> I known that gdb isn't at fault.  It can't do anything without the
> missing debug info.  The test was specifically written to use that
> debug info.

I'm not really sure what's the right thing to do here.

On the one hand, my current task is ensuring GDB can debug
clang-compiled with clang as well as it can debug GCC-compiled
code.  From that perspective the skip-if-clang logic in this
test is hiding a failure I need to investigate.

On the other hand, I'm an engineer working on GDB, and from that
perspective I want to be able to run the GDB testsuite and see
100% pass, on whatever setup I test it on.  And yes, I know it
doesn't... but it *should*.

Is there a way to pass a "don't skip clang failures" flag to the
testcases, such that people running the testsuite normally would
see tests like these return UNSUPPORTED, but I could run the
testsuite with the flag so it'd not skip but FAIL wherever the
problem is?

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-26  9:37             ` Gary Benson
@ 2020-06-28 10:50               ` Tom de Vries
  2020-06-29 12:32                 ` Pedro Alves
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-06-28 10:50 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 6/26/20 11:37 AM, Gary Benson wrote:
> Tom de Vries wrote:
>> On 6/19/20 4:00 PM, Gary Benson wrote:
>>> Tom de Vries wrote:
>>>> On 6/18/20 6:10 PM, Gary Benson wrote:
>>>>> Tom de Vries wrote:
>>>>>> On 6/17/20 2:24 PM, Gary Benson wrote:
>>>>>>> Tom, I'd like this testcase to not fail silently.  Is the
>>>>>>> functionality under test something that isn't ever
>>>>>>> expected to work with clang, or is this a test that should
>>>>>>> pass with clang (but it currently doesn't, for whatever
>>>>>>> reason)?
>>>>>>
>>>>>> I'm not sure.  The test can pass with clang, provided it
>>>>>> generates the required debug info.  It currently doesn't.
>>>>>> Why that is the case, I have no idea.
>>>>>
>>>>> I think that means the test should work but it doesn't.  Would
>>>>> you object if I push a patch removing the test-skipping logic?
>>>>> It will mean an extra FAIL when tested using clang
>>>>
>>>> I don't think having a fail for a compiler bug/missing-feature
>>>> is a good idea.
>>>>
>>>> If this is due to a bug/missing-feature in clang, then we need to:
>>>> - xfail the test,
>>>> - file the PR in clang, and
>>>> - reference the PR at the xfail.
>>>
>>> Is this a bug/missing feature in clang though?
>>> How sure are you GDB isn't at fault?
>>
>> Clang emits less debug info than GCC.  Whether that's a bug, a
>> missing feature or an explicit unsupported feature in clang, I
>> don't known.
>>
>> I known that gdb isn't at fault.  It can't do anything without the
>> missing debug info.  The test was specifically written to use that
>> debug info.
> 
> I'm not really sure what's the right thing to do here.
> 
> On the one hand, my current task is ensuring GDB can debug
> clang-compiled with clang as well as it can debug GCC-compiled
> code.  From that perspective the skip-if-clang logic in this
> test is hiding a failure I need to investigate.
> 
> On the other hand, I'm an engineer working on GDB, and from that
> perspective I want to be able to run the GDB testsuite and see
> 100% pass, on whatever setup I test it on.  And yes, I know it
> doesn't... but it *should*.
> 
> Is there a way to pass a "don't skip clang failures" flag to the
> testcases, such that people running the testsuite normally would
> see tests like these return UNSUPPORTED, but I could run the
> testsuite with the flag so it'd not skip but FAIL wherever the
> problem is?

I think the following is a good way of dealing with this.

We introduce a proc in gdb.exp called debug_info_for_decl or some such,
that returns false by default for clang.

Instead of testing for a specific compiler in the test-case,  we call
this new proc, and mark the test-case UNSUPPORTED if it returns false.

Then if you want to see how clang performs if we expect it to have that
feature, you comment out the clang-specific code in the proc.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-28 10:50               ` Tom de Vries
@ 2020-06-29 12:32                 ` Pedro Alves
  2020-06-30  8:14                   ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2020-06-29 12:32 UTC (permalink / raw)
  To: Tom de Vries, Gary Benson; +Cc: gdb-patches

On 6/28/20 11:50 AM, Tom de Vries wrote:
> On 6/26/20 11:37 AM, Gary Benson wrote:
>> Tom de Vries wrote:
>>> On 6/19/20 4:00 PM, Gary Benson wrote:
>>>> Tom de Vries wrote:
>>>>> On 6/18/20 6:10 PM, Gary Benson wrote:
>>>>>> Tom de Vries wrote:
>>>>>>> On 6/17/20 2:24 PM, Gary Benson wrote:
>>>>>>>> Tom, I'd like this testcase to not fail silently.  Is the
>>>>>>>> functionality under test something that isn't ever
>>>>>>>> expected to work with clang, or is this a test that should
>>>>>>>> pass with clang (but it currently doesn't, for whatever
>>>>>>>> reason)?
>>>>>>>
>>>>>>> I'm not sure.  The test can pass with clang, provided it
>>>>>>> generates the required debug info.  It currently doesn't.
>>>>>>> Why that is the case, I have no idea.
>>>>>>
>>>>>> I think that means the test should work but it doesn't.  Would
>>>>>> you object if I push a patch removing the test-skipping logic?
>>>>>> It will mean an extra FAIL when tested using clang
>>>>>
>>>>> I don't think having a fail for a compiler bug/missing-feature
>>>>> is a good idea.
>>>>>
>>>>> If this is due to a bug/missing-feature in clang, then we need to:
>>>>> - xfail the test,
>>>>> - file the PR in clang, and
>>>>> - reference the PR at the xfail.
>>>>
>>>> Is this a bug/missing feature in clang though?
>>>> How sure are you GDB isn't at fault?
>>>
>>> Clang emits less debug info than GCC.  Whether that's a bug, a
>>> missing feature or an explicit unsupported feature in clang, I
>>> don't known.
>>>
>>> I known that gdb isn't at fault.  It can't do anything without the
>>> missing debug info.  The test was specifically written to use that
>>> debug info.
>>
>> I'm not really sure what's the right thing to do here.
>>
>> On the one hand, my current task is ensuring GDB can debug
>> clang-compiled with clang as well as it can debug GCC-compiled
>> code.  From that perspective the skip-if-clang logic in this
>> test is hiding a failure I need to investigate.
>>
>> On the other hand, I'm an engineer working on GDB, and from that
>> perspective I want to be able to run the GDB testsuite and see
>> 100% pass, on whatever setup I test it on.  And yes, I know it
>> doesn't... but it *should*.
>>
>> Is there a way to pass a "don't skip clang failures" flag to the
>> testcases, such that people running the testsuite normally would
>> see tests like these return UNSUPPORTED, but I could run the
>> testsuite with the flag so it'd not skip but FAIL wherever the
>> problem is?
> 
> I think the following is a good way of dealing with this.
> 
> We introduce a proc in gdb.exp called debug_info_for_decl or some such,
> that returns false by default for clang.

I think it would be useful to include an intro description to

 gdb.base/psym-external-decl.exp

since it currently doesn't describe what it is testing.  Looking
at the commit log of the patch that introduced it helps, but
one shouldn't have to do that.

So the difference between gcc and clang AFAIU is that gcc emits
debug info for the extern variable declaration (and with newer GCCs,
only if there's a reference to the variable in the CU, otherwise
not even so), while seemingly clang would only emit debug info for
the variable's definition, which isn't compiled with -g, so we
end up with no debug info for the variable.

I would suggest filing a bug with clang, to confirm whether
this is intentional, or whether they see it as a bug.  I would
think it is a bug, but I'm not sure.  If indeed a bug, we would
XFAIL the test.

> 
> Instead of testing for a specific compiler in the test-case,  we call
> this new proc, and mark the test-case UNSUPPORTED if it returns false.
> 
> Then if you want to see how clang performs if we expect it to have that
> feature, you comment out the clang-specific code in the proc.
> 
> Thanks,
> - Tom
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-29 12:32                 ` Pedro Alves
@ 2020-06-30  8:14                   ` Tom de Vries
  2020-07-03  9:21                     ` Gary Benson
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-06-30  8:14 UTC (permalink / raw)
  To: Pedro Alves, Gary Benson; +Cc: gdb-patches

On 6/29/20 2:32 PM, Pedro Alves wrote:
> On 6/28/20 11:50 AM, Tom de Vries wrote:
>> On 6/26/20 11:37 AM, Gary Benson wrote:
>>> Tom de Vries wrote:
>>>> On 6/19/20 4:00 PM, Gary Benson wrote:
>>>>> Tom de Vries wrote:
>>>>>> On 6/18/20 6:10 PM, Gary Benson wrote:
>>>>>>> Tom de Vries wrote:
>>>>>>>> On 6/17/20 2:24 PM, Gary Benson wrote:
>>>>>>>>> Tom, I'd like this testcase to not fail silently.  Is the
>>>>>>>>> functionality under test something that isn't ever
>>>>>>>>> expected to work with clang, or is this a test that should
>>>>>>>>> pass with clang (but it currently doesn't, for whatever
>>>>>>>>> reason)?
>>>>>>>>
>>>>>>>> I'm not sure.  The test can pass with clang, provided it
>>>>>>>> generates the required debug info.  It currently doesn't.
>>>>>>>> Why that is the case, I have no idea.
>>>>>>>
>>>>>>> I think that means the test should work but it doesn't.  Would
>>>>>>> you object if I push a patch removing the test-skipping logic?
>>>>>>> It will mean an extra FAIL when tested using clang
>>>>>>
>>>>>> I don't think having a fail for a compiler bug/missing-feature
>>>>>> is a good idea.
>>>>>>
>>>>>> If this is due to a bug/missing-feature in clang, then we need to:
>>>>>> - xfail the test,
>>>>>> - file the PR in clang, and
>>>>>> - reference the PR at the xfail.
>>>>>
>>>>> Is this a bug/missing feature in clang though?
>>>>> How sure are you GDB isn't at fault?
>>>>
>>>> Clang emits less debug info than GCC.  Whether that's a bug, a
>>>> missing feature or an explicit unsupported feature in clang, I
>>>> don't known.
>>>>
>>>> I known that gdb isn't at fault.  It can't do anything without the
>>>> missing debug info.  The test was specifically written to use that
>>>> debug info.
>>>
>>> I'm not really sure what's the right thing to do here.
>>>
>>> On the one hand, my current task is ensuring GDB can debug
>>> clang-compiled with clang as well as it can debug GCC-compiled
>>> code.  From that perspective the skip-if-clang logic in this
>>> test is hiding a failure I need to investigate.
>>>
>>> On the other hand, I'm an engineer working on GDB, and from that
>>> perspective I want to be able to run the GDB testsuite and see
>>> 100% pass, on whatever setup I test it on.  And yes, I know it
>>> doesn't... but it *should*.
>>>
>>> Is there a way to pass a "don't skip clang failures" flag to the
>>> testcases, such that people running the testsuite normally would
>>> see tests like these return UNSUPPORTED, but I could run the
>>> testsuite with the flag so it'd not skip but FAIL wherever the
>>> problem is?
>>
>> I think the following is a good way of dealing with this.
>>
>> We introduce a proc in gdb.exp called debug_info_for_decl or some such,
>> that returns false by default for clang.
> 
> I think it would be useful to include an intro description to
> 
>  gdb.base/psym-external-decl.exp
> 
> since it currently doesn't describe what it is testing.  Looking
> at the commit log of the patch that introduced it helps, but
> one shouldn't have to do that.
> 
> So the difference between gcc and clang AFAIU is that gcc emits
> debug info for the extern variable declaration (and with newer GCCs,
> only if there's a reference to the variable in the CU, otherwise
> not even so), while seemingly clang would only emit debug info for
> the variable's definition, which isn't compiled with -g, so we
> end up with no debug info for the variable.
> 
> I would suggest filing a bug with clang, to confirm whether
> this is intentional, or whether they see it as a bug.  I would
> think it is a bug, but I'm not sure.  If indeed a bug, we would
> XFAIL the test.
> 

I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-06-30  8:14                   ` Tom de Vries
@ 2020-07-03  9:21                     ` Gary Benson
  2020-07-03 11:20                       ` Pedro Alves
  0 siblings, 1 reply; 16+ messages in thread
From: Gary Benson @ 2020-07-03  9:21 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Pedro Alves, gdb-patches

Tom de Vries wrote:
> On 6/29/20 2:32 PM, Pedro Alves wrote:
> > On 6/28/20 11:50 AM, Tom de Vries wrote:
> > > I think the following is a good way of dealing with this.
> > >
> > > We introduce a proc in gdb.exp called debug_info_for_decl or
> > > some such, that returns false by default for clang.
> > 
> > I think it would be useful to include an intro description to
> > 
> >  gdb.base/psym-external-decl.exp
> > 
> > since it currently doesn't describe what it is testing.  Looking
> > at the commit log of the patch that introduced it helps, but
> > one shouldn't have to do that.
> > 
> > So the difference between gcc and clang AFAIU is that gcc emits
> > debug info for the extern variable declaration (and with newer GCCs,
> > only if there's a reference to the variable in the CU, otherwise
> > not even so), while seemingly clang would only emit debug info for
> > the variable's definition, which isn't compiled with -g, so we
> > end up with no debug info for the variable.
> > 
> > I would suggest filing a bug with clang, to confirm whether
> > this is intentional, or whether they see it as a bug.  I would
> > think it is a bug, but I'm not sure.  If indeed a bug, we would
> > XFAIL the test.
> 
> I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .
> 
> Thanks,
> - Tom

Thank for filing it Tom.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-07-03  9:21                     ` Gary Benson
@ 2020-07-03 11:20                       ` Pedro Alves
  2020-07-03 11:24                         ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2020-07-03 11:20 UTC (permalink / raw)
  To: Gary Benson, Tom de Vries; +Cc: gdb-patches

On 7/3/20 10:21 AM, Gary Benson wrote:
> Tom de Vries wrote:
>> On 6/29/20 2:32 PM, Pedro Alves wrote:

>>> I would suggest filing a bug with clang, to confirm whether
>>> this is intentional, or whether they see it as a bug.  I would
>>> think it is a bug, but I'm not sure.  If indeed a bug, we would
>>> XFAIL the test.
>>
>> I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .
>>
>> Thanks,
>> - Tom
> 
> Thank for filing it Tom.

So the conclusion seems to be that the testcase should be using
-fstandalone-debug?

Except, I tried it here, and it doesn't seem to work:

~~~~~~~~~~~~~~~~
From 792aa03ab3325fe7cfcfeb51e765e81da2fbbfaf Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 3 Jul 2020 12:12:19 +0100
Subject: [PATCH] standalone

---
 gdb/testsuite/gdb.base/psym-external-decl.exp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
index d0388d5655e..faff3234bb5 100644
--- a/gdb/testsuite/gdb.base/psym-external-decl.exp
+++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
@@ -15,17 +15,18 @@
 
 standard_testfile .c psym-external-decl-2.c
 
+set srcfiles [list $srcfile $srcfile2]
+set options {debug}
+
 get_compiler_info
 if { [test_compiler_info "clang-*"] } {
-    return -1
+    lappend options additional_flags=-fstandalone-debug
 }
 
-set srcfiles [list $srcfile $srcfile2]
-
 if { [build_executable_from_specs \
 	  "failed to prepare" \
 	  $testfile [list] \
-	  $srcfile [list debug] \
+	  $srcfile $options \
 	  $srcfile2 [list]] == -1 } {
     return -1
 }

base-commit: c2ecccb33c307faa21f4d2f47348e7346b032d94
-- 
2.14.5
~~~~~~~~~~~~~~~~

I still get:

$ readelf -w testsuite/outputs/gdb.base/psym-external-decl/psym-external-decl0.o | grep aaa
$

This is Clang 5.0.2.  Maybe it works with newer Clangs?  What do you get?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-07-03 11:20                       ` Pedro Alves
@ 2020-07-03 11:24                         ` Tom de Vries
  2020-07-03 11:32                           ` Pedro Alves
  0 siblings, 1 reply; 16+ messages in thread
From: Tom de Vries @ 2020-07-03 11:24 UTC (permalink / raw)
  To: Pedro Alves, Gary Benson; +Cc: gdb-patches

On 7/3/20 1:20 PM, Pedro Alves wrote:
> On 7/3/20 10:21 AM, Gary Benson wrote:
>> Tom de Vries wrote:
>>> On 6/29/20 2:32 PM, Pedro Alves wrote:
> 
>>>> I would suggest filing a bug with clang, to confirm whether
>>>> this is intentional, or whether they see it as a bug.  I would
>>>> think it is a bug, but I'm not sure.  If indeed a bug, we would
>>>> XFAIL the test.
>>>
>>> I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .
>>>
>>> Thanks,
>>> - Tom
>>
>> Thank for filing it Tom.
> 
> So the conclusion seems to be that the testcase should be using
> -fstandalone-debug?
> 
> Except, I tried it here, and it doesn't seem to work:
> 

It is my understanding that -fstandalone-debug should be used for clang,
but that is doesn't work atm, which is a problem in clang/llvm that is
tracked in the PR.

Thanks,
- Tom

> ~~~~~~~~~~~~~~~~
> From 792aa03ab3325fe7cfcfeb51e765e81da2fbbfaf Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 3 Jul 2020 12:12:19 +0100
> Subject: [PATCH] standalone
> 
> ---
>  gdb/testsuite/gdb.base/psym-external-decl.exp | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/psym-external-decl.exp b/gdb/testsuite/gdb.base/psym-external-decl.exp
> index d0388d5655e..faff3234bb5 100644
> --- a/gdb/testsuite/gdb.base/psym-external-decl.exp
> +++ b/gdb/testsuite/gdb.base/psym-external-decl.exp
> @@ -15,17 +15,18 @@
>  
>  standard_testfile .c psym-external-decl-2.c
>  
> +set srcfiles [list $srcfile $srcfile2]
> +set options {debug}
> +
>  get_compiler_info
>  if { [test_compiler_info "clang-*"] } {
> -    return -1
> +    lappend options additional_flags=-fstandalone-debug
>  }
>  
> -set srcfiles [list $srcfile $srcfile2]
> -
>  if { [build_executable_from_specs \
>  	  "failed to prepare" \
>  	  $testfile [list] \
> -	  $srcfile [list debug] \
> +	  $srcfile $options \
>  	  $srcfile2 [list]] == -1 } {
>      return -1
>  }
> 
> base-commit: c2ecccb33c307faa21f4d2f47348e7346b032d94
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-07-03 11:24                         ` Tom de Vries
@ 2020-07-03 11:32                           ` Pedro Alves
  2020-07-03 12:50                             ` Tom de Vries
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2020-07-03 11:32 UTC (permalink / raw)
  To: Tom de Vries, Gary Benson; +Cc: gdb-patches

On 7/3/20 12:24 PM, Tom de Vries wrote:
> On 7/3/20 1:20 PM, Pedro Alves wrote:
>> On 7/3/20 10:21 AM, Gary Benson wrote:
>>> Tom de Vries wrote:
>>>> On 6/29/20 2:32 PM, Pedro Alves wrote:
>>
>>>>> I would suggest filing a bug with clang, to confirm whether
>>>>> this is intentional, or whether they see it as a bug.  I would
>>>>> think it is a bug, but I'm not sure.  If indeed a bug, we would
>>>>> XFAIL the test.
>>>>
>>>> I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .
>>>>
>>>> Thanks,
>>>> - Tom
>>>
>>> Thank for filing it Tom.
>>
>> So the conclusion seems to be that the testcase should be using
>> -fstandalone-debug?
>>
>> Except, I tried it here, and it doesn't seem to work:
>>
> 
> It is my understanding that -fstandalone-debug should be used for clang,
> but that is doesn't work atm, which is a problem in clang/llvm that is
> tracked in the PR.

I don't see anywhere in the PR stating that it's known that it doesn't work?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang
  2020-07-03 11:32                           ` Pedro Alves
@ 2020-07-03 12:50                             ` Tom de Vries
  0 siblings, 0 replies; 16+ messages in thread
From: Tom de Vries @ 2020-07-03 12:50 UTC (permalink / raw)
  To: Pedro Alves, Gary Benson; +Cc: gdb-patches

On 7/3/20 1:32 PM, Pedro Alves wrote:
> On 7/3/20 12:24 PM, Tom de Vries wrote:
>> On 7/3/20 1:20 PM, Pedro Alves wrote:
>>> On 7/3/20 10:21 AM, Gary Benson wrote:
>>>> Tom de Vries wrote:
>>>>> On 6/29/20 2:32 PM, Pedro Alves wrote:
>>>
>>>>>> I would suggest filing a bug with clang, to confirm whether
>>>>>> this is intentional, or whether they see it as a bug.  I would
>>>>>> think it is a bug, but I'm not sure.  If indeed a bug, we would
>>>>>> XFAIL the test.
>>>>>
>>>>> I've filed https://bugs.llvm.org/show_bug.cgi?id=46514 .
>>>>>
>>>>> Thanks,
>>>>> - Tom
>>>>
>>>> Thank for filing it Tom.
>>>
>>> So the conclusion seems to be that the testcase should be using
>>> -fstandalone-debug?
>>>
>>> Except, I tried it here, and it doesn't seem to work:
>>>
>>
>> It is my understanding that -fstandalone-debug should be used for clang,
>> but that is doesn't work atm, which is a problem in clang/llvm that is
>> tracked in the PR.
> 
> I don't see anywhere in the PR stating that it's known that it doesn't work?

Um, perhaps I misinterpreted.  I've asked for clarification.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-07-03 12:51 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-02  7:51 [committed][gdb/testsuite] Update psym-external-decl.exp for gcc-10/clang Tom de Vries
2020-06-17 12:24 ` Gary Benson
2020-06-17 13:56   ` Tom de Vries
2020-06-18 16:10     ` Gary Benson
2020-06-18 16:27       ` Tom de Vries
2020-06-19 14:00         ` Gary Benson
2020-06-19 14:06           ` Tom de Vries
2020-06-26  9:37             ` Gary Benson
2020-06-28 10:50               ` Tom de Vries
2020-06-29 12:32                 ` Pedro Alves
2020-06-30  8:14                   ` Tom de Vries
2020-07-03  9:21                     ` Gary Benson
2020-07-03 11:20                       ` Pedro Alves
2020-07-03 11:24                         ` Tom de Vries
2020-07-03 11:32                           ` Pedro Alves
2020-07-03 12:50                             ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).