Coccinelle archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] RFC catching hidden code in if conditions
@ 2019-03-25 13:03 Nicholas Mc Guire
  2019-03-25 13:14 ` Julia Lawall
  2019-03-26 19:04 ` Markus Elfring
  0 siblings, 2 replies; 3+ messages in thread
From: Nicholas Mc Guire @ 2019-03-25 13:03 UTC (permalink / raw)
  To: Cocci


HI !

 Noticed that the nuveau driver uses a fair number of if (var=val,boolean-condition){}
 which while legal C-code just makes it hard to read - and some seems buggy 
 actually. The below spatch find about 50 cases - not sure though if this is
 actually complete ?

thx!
hofrat

/// Check for unconditional code "hiding" in an if condition
/// effectively that code is unconditionally executed before
/// reaching the actual branch statement - which just makes it
/// hard to read and thus is always wrong.
/// Some of the cases found also look buggy  
/// In other cases some excess () are left in place in the
/// generated patches - so some postprocessing may be needed.
///
/// As of 5.0-rc8 all 50 cases look like they are found and fixed
/// correctly - incorrectly only in the sense that the patched 
/// code is equivalent to the original code. but as this is in
/// the nouveau driver only it might well be that this only
/// fits that specific pattern and others might have wilder ways
/// to achieve the same - so confidence Low for now
///
// Confidence: Low
// Comments:
// Options: --no-includes --include-headers

virtual patch
virtual report

@badif@
position p;
statement S;
expression E1,E2;
@@

  if@p (E1,E2) S

@script:python depends on report@
p << badif.p;
@@

msg = "unconditional code hiding in if condition" 
coccilib.report.print_report(p[0],msg)

@fixbadif depends on badif && patch@
position p=badif.p;
statement S;
expression E1=badif.E1,E2=badif.E2;
@@

+ E1;
  if@p (
- E1,
  E2) 
  S 

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] RFC catching hidden code in if conditions
  2019-03-25 13:03 [Cocci] RFC catching hidden code in if conditions Nicholas Mc Guire
@ 2019-03-25 13:14 ` Julia Lawall
  2019-03-26 19:04 ` Markus Elfring
  1 sibling, 0 replies; 3+ messages in thread
From: Julia Lawall @ 2019-03-25 13:14 UTC (permalink / raw)
  To: Nicholas Mc Guire; +Cc: Cocci



On Mon, 25 Mar 2019, Nicholas Mc Guire wrote:

>
> HI !
>
>  Noticed that the nuveau driver uses a fair number of if (var=val,boolean-condition){}
>  which while legal C-code just makes it hard to read - and some seems buggy
>  actually. The below spatch find about 50 cases - not sure though if this is
>  actually complete ?

Maybe it would be good to forward it to the nouveau people and see what
they think about it?  I think that checkpatch already complains about
assignments in if conditions, and the whole thing with the comma makes it
look even more unpleasant.

julia

>
> thx!
> hofrat
>
> /// Check for unconditional code "hiding" in an if condition
> /// effectively that code is unconditionally executed before
> /// reaching the actual branch statement - which just makes it
> /// hard to read and thus is always wrong.
> /// Some of the cases found also look buggy
> /// In other cases some excess () are left in place in the
> /// generated patches - so some postprocessing may be needed.
> ///
> /// As of 5.0-rc8 all 50 cases look like they are found and fixed
> /// correctly - incorrectly only in the sense that the patched
> /// code is equivalent to the original code. but as this is in
> /// the nouveau driver only it might well be that this only
> /// fits that specific pattern and others might have wilder ways
> /// to achieve the same - so confidence Low for now
> ///
> // Confidence: Low
> // Comments:
> // Options: --no-includes --include-headers
>
> virtual patch
> virtual report
>
> @badif@
> position p;
> statement S;
> expression E1,E2;
> @@
>
>   if@p (E1,E2) S
>
> @script:python depends on report@
> p << badif.p;
> @@
>
> msg = "unconditional code hiding in if condition"
> coccilib.report.print_report(p[0],msg)
>
> @fixbadif depends on badif && patch@
> position p=badif.p;
> statement S;
> expression E1=badif.E1,E2=badif.E2;
> @@
>
> + E1;
>   if@p (
> - E1,
>   E2)
>   S
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] RFC catching hidden code in if conditions
  2019-03-25 13:03 [Cocci] RFC catching hidden code in if conditions Nicholas Mc Guire
  2019-03-25 13:14 ` Julia Lawall
@ 2019-03-26 19:04 ` Markus Elfring
  1 sibling, 0 replies; 3+ messages in thread
From: Markus Elfring @ 2019-03-26 19:04 UTC (permalink / raw)
  To: Nicholas Mc Guire; +Cc: cocci

> Noticed that the nuveau driver uses a fair number of
> if (var=val,boolean-condition){}
> which while legal C-code just makes it hard to read
> - and some seems buggy actually.

Do you find another analysis approach nicer for the semantic patch language
than the existing check “ASSIGN_IN_IF” by a known Perl script?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl?id=a3ac7917b73070010c05b4485b8582a6c9cd69b6#n4847

https://systeme.lip6.fr/pipermail/cocci/2018-September/005316.html


I suggest to take another look at recurring implementation details
also within SmPL scripts.

1. SPDX identifier

2. SmPL dependency specifications

3. Fine-tuning for Python code


@bad_if depends on report@
position P;
statement S1, S2;
expression E1, E2;
@@
 if@P ((E1), E2) S1 else S2

@script:python depends on report@
p << bad_if.P;
@@
coccilib.report.print_report(p[0],
                             "unconditional code hiding in if condition")

@fix_bad_if depends on patch@
position P;
statement S1, S2;
expression E1, E2;
@@
+E1;
 if@P (
-      (E1),
       E2) S1
 else S2

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 13:03 [Cocci] RFC catching hidden code in if conditions Nicholas Mc Guire
2019-03-25 13:14 ` Julia Lawall
2019-03-26 19:04 ` Markus Elfring

Coccinelle archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr cocci@archiver.kernel.org
	public-inbox-index cocci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox