Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen.5i5j@gmail.com>
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: palves@redhat.com, ibuclaw@gdcproject.org, brobecker@adacore.com,
	 gdb-patches@sourceware.org
Subject: Re: [PATCH v3] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"
Date: Sat, 25 Oct 2014 09:44:00 -0000	[thread overview]
Message-ID: <544B727B.1090301@gmail.com> (raw)
In-Reply-To: <201410250912.s9P9CHj4001707@glazunov.sibelius.xs4all.nl>

On 10/25/14 17:12, Mark Kettenis wrote:
>> Date: Sat, 25 Oct 2014 07:01:49 +0800
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> Hell Maintainers:
>>
>> Is this patch OK, if need additional improvements, please let me know.
>>
>> By the way: for "I387_MXCSR_REGNUM", I guess, gcc 'think' it is for 2
>> variables, which does not match "(X + c) >= X" ('c' means constant, I
>> guess), so gcc does not report warning for it (then I did not touch it).
> 
> No this patch is not ok.  It doesn't implement Pedro's suggestion to
> rewrite the loops.  I started working on that, but then I discovered
> that there are many more similar loops where your compiler apparently
> doesn't warn about signed overflow in the comparison.  Perhaps I'll
> finish my diff some day, but it isn't a very high priority for me.
> 

It seems a misunderstanding, for me, Pedro's suggestion is also for
avoiding compiling warnings, and his idea is better than others.

And just like you said, there are many almost the same using ways within
this file, but gcc5 does not report warnings for them.

> I don't really want to uglify the code just to make unhelpful
> compilers happy.  Playing whack-a-mle with GCC isn't my idea of fun.
> 

Neither me. But we can not say that is GCC5' issue, for me:

 - "(X + c) >= X" may really find some issues which developers missed,
   so it is still valuable.

 - gcc treates their warnings are only as the 'advice' for developers,
   not mandatory. 'advice' must be valuable, but may be not suitable in
   any cases.

 - but our gdb wants to treate all 'advice' as mandatory whether it is
   suitable for us or not.

So we have to try to let our code to be 'suitable' for both us and GCC5.

> And yes, your compiler is being unhelpful.  If it warns about possible
> signed overflow in the RHS expression of a comparision, why doesn't it
> warn about any signed addition that might overflow?
> 

For me, because they are not match "(X + c) >= X" ('c' is constant). For
"-Wstrict-overflow", it includes "(X + c) >= X", but not others.  :-(

And can we disable "-Wstrict-overflow" (I guess not)?

And there is another clearer way, although it may be even more uglier:
"#pragma diagnostic ..."


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed


      reply	other threads:[~2014-10-25  9:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-16 12:02 Chen Gang
2014-10-24 22:55 ` Chen Gang
2014-10-25  9:12   ` Mark Kettenis
2014-10-25  9:44     ` Chen Gang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=544B727B.1090301@gmail.com \
    --to=gang.chen.5i5j@gmail.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=ibuclaw@gdcproject.org \
    --cc=mark.kettenis@xs4all.nl \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox