Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] False positives with Codan using Avr-gcc toolchain

Normally I would agree, however branch tests are of such low cost, I can't see the impact being significant (even for low end machines) unless there's some aspect of the java runtime that makes these branch tests of a high performance cost

I did however decide that there is a problem with the logic: if the statement being tested is not the last in the list of a statement-_expression_, then it could be a valid "statement has no effect" warning.

I don't know much about the object model, but if it's possible to identify that the statement is the last in it's block, AND it's a part of a statement-_expression_, then ignore it, otherwise, make normal checks - I will look into modifying the patch accordingly and see if I can add a test


On Thu, Apr 24, 2014 at 8:54 PM, Alena Laskavaia <elaskavaia.cdt@xxxxxxxxx> wrote:
Btw moving this check up will have a performance impact, it is better to check this condition when we found something to report


On Thu, Apr 24, 2014 at 12:10 PM, Nathan Ridge <zeratul976@xxxxxxxxxxx> wrote:
Hi Chris,

Thanks for writing the patch, it looks good!

Could you submit it via Gerrit as described here [1]? Thanks!

If you wanted to add a test to StatementHasNoEffectCheckerTest.java
as well, that would round things out nicely.

Regards,
Nate

[1] http://wiki.eclipse.org/CDT/git#Using_Gerrit_for_CDT

________________________________
> Date: Thu, 24 Apr 2014 13:06:19 +0100
> From: cdabone@xxxxxxxxx
> To: cdt-dev@xxxxxxxxxxx
> Subject: Re: [cdt-dev] False positives with Codan using Avr-gcc toolchain
>
> I've dug out the codan analyser source and looked thought about it
> some, based on what you've said [Nathan] - it seems to me that the
> logic for this scenario can never flag true for a statement-_expression_,
> as all C operations will return either void (valid for
> statement-expressions according to the manual) or otherwise be valid
> R-values. The only other thing to check for is perhaps a void return
> from a statement-_expression_, being assigned to an Lvalue, but that will
> show up as a compile error (I believe, I may be wrong...), rather than
> a "statement has no effect" type bug.
>
> Based on that, I've made some adjustments to the .java file for that
> checker and attached a patch file (apologies if it's not perfect, I'm
> on a windows machine at the moment behind a troublesome proxy, I've
> copied and pasted the source into a.txt file and used that to generate
> the patch)
>
> Chris
>
>
>
> On Thu, Apr 24, 2014 at 12:22 PM, Chris Marr
> <cdabone@xxxxxxxxx<mailto:cdabone@xxxxxxxxx>> wrote:
> Thanks muchly Nathan, Statement-Expressions are new to me (being very
> much tied into historic and non-GNU compilers in my day job) so as they
> say, every day is school day.
>
> Thanks for the feedback
> Chris
>
>
> On Thu, Apr 24, 2014 at 8:30 AM, Nathan Ridge
> <zeratul976@xxxxxxxxxxx<mailto:zeratul976@xxxxxxxxxxx>> wrote:
> Hi Chris,
>
> This actually looks like a bug in Codan.
>
> Expressions of the form
>
>      ({ <statements> })
>
> are a GCC extension called "statement-expressions" [1]. Their
> semantics is that they evaluate to the value of the _expression_
> in the last statement (or void if the last statement is not an
> _expression_ statement).
>
> ('__extension__' is just a GCC builtin that prevents certain
> warnings from being issued [2]. CDT treats it as a macro with
> an empty expansion.)
>
> It is therefore incorrect to give a "statement has no effect"
> warning for the last statement in a statement-_expression_, since
> in this case the value of the _expression_ in the statement is
> actually used.
>
> Codan already has some logic to avoid giving this warning in
> such cases, but it's buggy and some cases like yours slip
> through the cracks. I filed a bug for this [3] and will provide
> a patch for it soon.
>
> Regards,
> Nate
>
> [1] http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
> [2] http://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html
> [3] https://bugs.eclipse.org/bugs/show_bug.cgi?id=433357
>
> ________________________________
> > Date: Wed, 23 Apr 2014 23:56:30 +0100
> > From: cdabone@xxxxxxxxx<mailto:cdabone@xxxxxxxxx>
> > To: cdt-dev@xxxxxxxxxxx<mailto:cdt-dev@xxxxxxxxxxx>
> > Subject: [cdt-dev] False positives with Codan using Avr-gcc toolchain
> >
> > Hi,
> >
> > I've just started developing with Eclipse and the Avr-GCC (WinAVR)
> > toolchain and I get a few false positives with codan. They're
> > irritating and out of harmony (I like clean code) so I'm trying to
> > alleviate them where possible. For those that don't know, one aspect of
> > Avr-GCC is a flash memory management library that uses some C
> > extensions to move RAM type initialisation into flash (avr/pgmspace.h
> > header) and manage it all via a set of macros. One specific macro for
> > managing strings (so you can pass info back and forth through a serial
> > port etc) is PSTR(s) which expands thus:
> >
> > # define PSTR(s) (__extension__({static char __c[] PROGMEM = (s);
> &__c[0];}))
> >
> > That "&__c[0];" is getting interpretted as a Codan warning "Statement
> > has no effect {0}" when it's actually the return value of the PSTR
> > macro (why not just regular macros? why have __extension__ in there? If
> > it needs to be a function, be a useful function and return the pointer
> > it's generating)
> >
> > I'd like to supress it either by fixing the avr toolchain (beyond me
> > today) or supress the error in this instance (add an exception to
> > codan's generation of this message).
> >
> > So 1) does codan unwrap macros before evaluating exceptions?
> >
> > I'd like to add an exception to the error that filters out the
> > _expression_: "(__extension({*}))" or should I filter "PSTR(*)"?
> >
> > I'm new to the "__extension__" keyword, so I'm not entirely sure how
> > that's evaluated in avr-gcc (and if someone says lmgtfy I'll slap them,
> > because if it was that simple, i wouldn't be here asking the question)
> > - the manual's a little thin in that regard
> >
> > in anticipation
> >
> > Chris
> >
> > _______________________________________________ cdt-dev mailing list
> > cdt-dev@xxxxxxxxxxx<mailto:cdt-dev@xxxxxxxxxxx>
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx<mailto:cdt-dev@xxxxxxxxxxx>
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>
>
>
> _______________________________________________ cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/cdt-dev

_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev


_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev



Back to the top