Bug 432433

Summary: Bug in factory code checks for sequence-point
Product: [openSUSE] openSUSE.org Reporter: Stephan Kleine <bitdealer>
Component: BuildServiceAssignee: Richard Biener <rguenther>
Status: RESOLVED INVALID QA Contact: Adrian Schröter <adrian.schroeter>
Severity: Normal    
Priority: P4 - Low CC: andrea, dmueller, mrueckert
Version: unspecified   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Bug Depends on: 444188    
Bug Blocks:    
Attachments: The preprocessed source
The preprocessed source - 2nd try
unreduced testcase

Description Stephan Kleine 2008-10-05 21:38:22 UTC 
My Code::Blocks build for Factory fails because of the following:

I: Program causes undefined operation
(likely same variable used twice and post/pre incremented in the same expression).
e.g. x = x++; Split it in two operations.
E: codeblocks sequence-point ../../../src/include/filefilters.h: 208

See https://build.opensuse.org/package/live_build_log?arch=i586&package=CodeBlocks&project=home%3Abitshuffler&repository=openSUSE_Factory for the full log.

The file in question is http://pastebin.ca/1220176 .

As you can see the file isn't 208 lines long and a header file, which is why I was told that the check might report the wrong file which sounds pretty reasonable to me (I'm absolutely no C/C++ guy).

Also the only option to find out what triggered that check to fail was said to be a complete code review starting with every file that includes that header file. Which isn't really an option either since the codebase is pretty big.


So I would like to suggest the following:

1. If a check fails the program should cite the offending line of code so I can at least narrow it down by greping for that line.
2. If the check needs only one line as input it should rerun against the extracted / cited line of code.
3. The check should grep the source file for the offending line of code to verify its extracted filename and line number are correct.

While only the first thing is relevant to me (the user) to find the offending code and fix it the other two would be great IMHO to verify the correctness of the checks since they would act like unit tests for every single failed check and thereby help you improving them.
Comment 1 Adrian Schröter 2008-10-06 06:42:58 UTC 
For Rudi, but he is on vacation this week.
Comment 2 andrea florio 2008-10-09 21:27:52 UTC 
similar issue...

error is:

I: Program is using implicit definitions of special functions.
these functions need to use their correct prototypes to allow
the lightweight buffer overflow checking to work.
- Implicit memory/string functions need #include <string.h>.
- Implicit *printf functions need #include <stdio.h>.
- Implicit *printf functions need #include <stdio.h>.
- Implicit *read* functions need #include <unistd.h>.
- Implicit *recv* functions need #include <sys/socket.h>.
E: OpenCASCADE implicit-fortify-decl ../../../drv/ExprIntrp/lex.ExprIntrp.c: 2618

and file ../../../drv/ExprIntrp/lex.ExprIntrp.c is that one http://pastebin.com/f695fc969

the error tell me to add the following somewhere..

#include <string.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/socket.h>

but as you can see stdio.h is already included (line 29), same for unistd.h (line 43)

full log here

https://build.opensuse.org/package/show?package=OpenCascade&project=home%3Aanubisg1
Comment 3 Marcus Rückert 2008-10-09 21:30:50 UTC 
just look for the offending filename earlier in the build log
and you will get a clue what is missing. the error message just says it is implict declarations. and mentions some common failures.
Comment 4 andrea florio 2008-10-10 21:56:57 UTC 
looks to be a false positive:

http://www.opencascade.org/org/forum/thread_14377/ (last comment by Denis Barbier)

no way to suppress it?
Comment 5 Ruediger Oertel 2008-10-14 23:57:50 UTC 
gcc warning checks ... calling for help ;)
Comment 6 Dirk Mueller 2008-10-15 01:20:06 UTC 
the quoted forum link contains the patch to fix the warning. another solution would be to use "-w" when building that particular file. 

unfortunately I couldn't figure out what the actual warning was because the buildservice project fails due to FHS file checks at the moment and not due to something else. 

to avoid that, remove the post-build-checks package (BuildIgnore: could do it I think, or BuildRequires: -post-build-checks).
Comment 7 Dirk Mueller 2008-10-15 01:26:31 UTC 
regarding comment one: 
  the line in question is: 

   id = (id == wxNOT_FOUND) ? 7 : ++id;

however, the diagnostic location is indeed wrong. looks like a bug
Comment 8 Richard Biener 2008-10-15 08:07:14 UTC 
We need a testcase (preprocessed source of the offending file) to do anything
about it.
Comment 9 Stephan Kleine 2008-10-15 13:58:58 UTC 
Sorry if I misunderstand you but I already posted the source in question (pastebin link in the first post) which contains nothing that would justify such an error so it is probably the wrong file (which is why I asked you to change the tests so they print out the line of code that made the check fail so I at least could grep for it).

Besides that the whole stuff (source & build logs) are available from https://build.opensuse.org/package/show?package=CodeBlocks&project=home%3Abitshuffler (sorry for the inconvenience but I don't know a thing about C/C++).
Comment 10 Michael Matz 2008-10-29 14:07:48 UTC 
If comment #7 indicates the line that GCC warns about, then GCC is correct:

  id = (id == wxNOT_FOUND) ? 7 : ++id;

There's a sequence point after the evaluation of the conditional, but not
any more.  Hence, if the condition is false, this is equivalent to:

  id = ++id;

which indeed is invalid C and C++.  I don't know if the rpmlint messages are
now fixed to indicate the right line number or not, but this is no GCC bug.
Reassigning to Dirk for the line number problem, he might close this if
that is fixed.
Comment 11 Dirk Mueller 2008-11-03 21:05:56 UTC 
the line number that the gcc gives as warning is incorrect. its not a bug in rpmlint/build check.
Comment 13 Richard Biener 2008-11-04 09:34:38 UTC 
We need preprocessed source to do anything about this.
Comment 14 Stephan Kleine 2008-11-04 15:36:54 UTC 
Created attachment 249684 [details]
The preprocessed source

The attached files should provide the requested information.

If they don't, please tell me _exactly_ how to retrieve what you want.
Comment 15 Richard Biener 2008-11-04 16:20:03 UTC 
The preprocessed file has been built with a precompiled header.  Can you
make sure to delete ../../../src/include/sdk_precomp.h.gch and undefine
CB_PRECOMP before preprocessing?  Basically the preprocessed source (the .ii
file) should not contain lines like

#pragma GCC pch_preprocess "../../../src/include/sdk_precomp.h.gch"

Thanks.
Richard.
Comment 16 Stephan Kleine 2008-11-04 19:38:09 UTC 
Created attachment 249753 [details]
The preprocessed source - 2nd try

I deleted ../../../src/include/sdk_precomp.h.gch and added "#undef CB_PRECOMP" at the top of projectsimporter.cpp and now there aren't any such lines in the .ii file.

If this is still not what you are looking for then please describe it in an even more brain dead / fool proof way because I don't really know what I am doing there ;D
Comment 17 Richard Biener 2008-11-05 09:13:44 UTC 
Created attachment 249886 [details]
unreduced testcase

Massaged testcase to build with any G++ version and for 32bit and 64bit.
Comment 18 Richard Biener 2008-11-05 10:48:22 UTC 
With the preprocessed testcase the line-number information is correct.  So the
bug is likely precompiled header related.
Comment 19 Richard Biener 2009-02-24 12:59:20 UTC 
The precompiled header has to be included from the primary source file or
via -include.