[RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Remove unused variable in gdb/varobj.c when built without Python support

Yao Qi
On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <[hidden email]> wrote:
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <[hidden email]>
>
>         * varobj.c (varobj_value_get_print_value): Move "gdbarch" to block
>         scope that uses it.

It is good to me.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Remove unused variable in windows-nat.c

Yao Qi
In reply to this post by Pedro Alves-7
On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <[hidden email]> wrote:
> Leave the call for side effects.
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <[hidden email]>
>
>         * windows-nat.c (handle_exception): Remove "th".

It is good to me.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Remove unused variable in gdb/varobj.c when built without Python support

Pedro Alves-7
In reply to this post by Yao Qi
On 07/21/2016 03:01 PM, Yao Qi wrote:
> On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <[hidden email]> wrote:
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <[hidden email]>
>>
>>         * varobj.c (varobj_value_get_print_value): Move "gdbarch" to block
>>         scope that uses it.
>
> It is good to me.
>

Pushed.

Thanks,
Pedro Alves

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Remove unused variable in windows-nat.c

Pedro Alves-7
In reply to this post by Yao Qi
On 07/21/2016 03:03 PM, Yao Qi wrote:

> On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <[hidden email]> wrote:
>> Leave the call for side effects.
>>
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <[hidden email]>
>>
>>         * windows-nat.c (handle_exception): Remove "th".
>
> It is good to me.
>

Pushed.

Thanks,
Pedro Alves

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Pedro Alves-7
In reply to this post by Pedro Alves-7
On 07/21/2016 01:15 PM, Pedro Alves wrote:

> On 07/21/2016 12:56 PM, Pedro Alves wrote:
>> On 07/21/2016 12:35 PM, Pedro Alves wrote:
>>> On 07/21/2016 12:10 PM, Yao Qi wrote:
>>>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <[hidden email]> wrote:
>>>
>>>>>
>>>>> I'd think it'd be acceptable to just build on a couple of the
>>>>> more common hosts, in the name of forward progress.
>>>>>
>>>>
>>>> Yes, I agree.
>>>>
>>>
>>> I had a mingw-w64 build tree already handy, so I gave it a try.  It
>>> needs a couple patches.  I'll send them as reply to this email.
>>
>> I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
>> now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
>> and no new warnings appeared.
>
> djgpp [1] has a few pre-existing build errors due to C++, but after
> fixing those, -Wunused-but-set-parameter -Wunused-but-set-variable
> introduce no new problems.

AIX (gcc111 on the compile farm) compiles fine.  It fails to link
due to the stabs overflow problem, but that's a preexisting problem.

Thanks,
Pedro Alves

>
> [1] - Installed gcc 6.1 from here ftp://ftp.delorie.com/pub/djgpp/rpms/
>
> Thanks,
> Pedro Alves
>

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Pedro Alves-7
On 07/21/2016 03:47 PM, Pedro Alves wrote:

> On 07/21/2016 01:15 PM, Pedro Alves wrote:
>> On 07/21/2016 12:56 PM, Pedro Alves wrote:
>>> On 07/21/2016 12:35 PM, Pedro Alves wrote:
>>>> On 07/21/2016 12:10 PM, Yao Qi wrote:
>>>>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <[hidden email]> wrote:
>>>>
>>>>>>
>>>>>> I'd think it'd be acceptable to just build on a couple of the
>>>>>> more common hosts, in the name of forward progress.
>>>>>>
>>>>>
>>>>> Yes, I agree.
>>>>>
>>>>
>>>> I had a mingw-w64 build tree already handy, so I gave it a try.  It
>>>> needs a couple patches.  I'll send them as reply to this email.
>>>
>>> I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
>>> now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
>>> and no new warnings appeared.
>>
>> djgpp [1] has a few pre-existing build errors due to C++, but after
>> fixing those, -Wunused-but-set-parameter -Wunused-but-set-variable
>> introduce no new problems.
>
> AIX (gcc111 on the compile farm) compiles fine.  It fails to link
> due to the stabs overflow problem, but that's a preexisting problem.

No build regressions on

 PPC64 GNU/Linux
 S390 GNU/Linux
 x64-64 NetBSD

Any host missing?  I think we're good to go.

Thanks,
Pedro Alves

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Yao Qi
On Thu, Jul 21, 2016 at 4:18 PM, Pedro Alves <[hidden email]> wrote:
> No build regressions on
>
>  PPC64 GNU/Linux
>  S390 GNU/Linux
>  x64-64 NetBSD
>
> Any host missing?  I think we're good to go.
>

I don't think of any.  Yes, please :)

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

Tom Tromey-2
>>>>> "Yao" == Yao Qi <[hidden email]> writes:

Yao> On Thu, Jul 21, 2016 at 4:18 PM, Pedro Alves <[hidden email]> wrote:
>> No build regressions on
>>
>> PPC64 GNU/Linux
>> S390 GNU/Linux
>> x64-64 NetBSD
>>
>> Any host missing?  I think we're good to go.
>>

Yao> I don't think of any.  Yes, please :)

I'll push the patch in momentarily.  Feel free to ping me on any build
problems.  I can't test them but I am happy to write a patch for someone
else to try.


There are a few more warnings that would be good to enable.

-Wmisleading-indentation (for me gdb is already clean here)

-Wduplicated-cond
-Wtautological-compare

I have a patch for a couple of problems pointed out by this, but there
is a larger one, https://sourceware.org/bugzilla/show_bug.cgi?id=20362
- a pretty big bug in arm_record_vfp_data_proc_insn.

It would be handy to have a "try" server on the buildbot to test patches
like this.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Maciej W. Rozycki-3
In reply to this post by Paul_Koning
On Wed, 20 Jul 2016, [hidden email] wrote:

> > Overall with recent and less so improvements to GCC's and other
> > compilers' optimizers I think these heuristic unwinders have hardly any
> > value nowadays, they seem unable to get through function prologues
> > containing arbitrary instructions thrown there by the scheduler.  This is
> > very annoying in a common case where you interrupt a debuggee in the
> > middle of a sleeping syscall, with no way to backtrace through stripped
> > system shared libraries.
>
> My experience is that the heuristic unwinders can be made to handle a
> lot of what's thrown at them now, but it takes quite a lot of extra
> heuristics to do so.  I have much of this on an internal version.  
> Should I look into making them available?

 Absolutely!

> One thing I've done that may not be generally interesting: make the
> unwinders work in the kernel (NetBSD) and able to unwind across
> exception frames so you can use kernel debugging and see the stack all
> the way into the calling process.  I haven't found this all that
> interesting in online debugging, but it has sometimes been useful in
> analyzing kernel crash dumps.

 I think it's a separate matter -- and for post-mortem debugging (and even
live debugging e.g. with QEMU's integrated debug stub or if you're a lucky
one who has JTAG probe hardware to hand) you can actually implement an
exception frame sniffer/unwinder so that GDB can examine it automagically.  
As an example see mips-sde-tdep.c, handling exception frames from the old
Algorithmics/MTI SDE toolkit/board support package.  If you'd like to add
a similar handler for the NetBSD kernel, then I'll gladly accept it.

 In a typical user app debug scenario you have cases where you want to
interrupt the program and see where it is, examine its local state, often
in the function that made the C library call which ended up in the syscall
just interrupted.  In the absence of either debug information all the way
through the syscall's entry point or special support it does not work
however with ABIs such as these used with the MIPS processor, where the
structure of the stack frame is variable and you cannot backtrace by just
taking the value from the frame pointer register and using it recursively
to fetch previous frame pointers from the stack.

 Unlike say with x86 or Power, where you may not be able to get complete
information about the innermost or some intermediate frames, but at least
you can backtrace far enough to reach a frame associated with a function
from your actual program being debugged and be able to fully examine the
state there as well as within any previous frames.

 Based on my personal experience with debugging software I think it is a
serious shortcoming of the MIPS backend, so if you have a way to improve
the current situation, then by any means please share it and make people's
life easier.  It'll be a huge step forward even if sometime in the future
we may get ourselves other means, such as what I have outlined in my
previous e-mail or maybe yet something else.

  Maciej
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Maciej W. Rozycki-3
In reply to this post by Tom Tromey-2
On Wed, 20 Jul 2016, Tom Tromey wrote:

> Maciej>  Of course one could argue that keeping broken code (though in a manner
> Maciej> harmless to irrelevant cases) has little value, but at least it serves as
> Maciej> a reminder to do something about it sometime.
>
> I think on the whole it's preferable to file a bug for such cases; or to
> comment out the offending code and put in an explanation.  The warnings
> catch real bugs; but they are not as useful if they are noisy.

 Good point, I have filed PR tdep/20406 to track this issue.

  Maciej
123