Community
Participate
Working Groups
In a project that is using a custom scanner discovery profile, customizing any source file (property) results in the default SD profile being used for that file. 1. Add a custom scanner discovery profile that is exactly the same as org.eclipse.cdt.managedbuilder.core.GCCManagedMakePerProjectProfileC but which specifies -Dwhatever in its commandline. This involves some small additions/modifications to various plugin.xmls. I've attached a diff that details those changes. The diff includes a println in CommandLauncher.java that will help us see what's going on. 2. Launch a runtime workbench 3. Create a Hello World ANSI C project and add two c files that each have only #ifdef whatever // yadayada #endif Also add these three lines to main.c 4. Bring up the Discovery Options properties page and switch the discovery profile to the "myprofile" one 5. Build the project. In the console (in the debugger Eclipse instance), you should see a single scanner discovery execution, courtesy of the println we added in step 1. Launching: gcc -E -P -v -Dwhatever -dD C:/<somepath>/specs.c Open each of the source files and note that the #ifdef whatever/#endif's are not grayed. So far so good. Our custom SD profile defines "whatever" in its cmdline, thus the observed behavior is what we expect. 6. Bring up the properties for any file in the project. Make some innocuous change like enabling "Verbose" in C/C++ Buid > Settings > Tool Settings > Miscellaneous 7. Clear the Console in the debugging Eclipse instance, then back in the runtime workbench clean and build the project You will now see two invocations of scanner discovery Launching: gcc -E -P -v -Dwhatever -dD C:/<somepath>/specs.c Launching: gcc -E -P -v -dD C:/<somepath>/specs.c Note that one lacks the -Dwhatever switch. This tells us that the additional SD invocation is based on the stock SD profile instead of our custom one. This is unexpected since we haven't changed any settings to do with scanner discovery in step 6. If you bring up the file you customized in the editor, you'll now see that #ifdef whatever/#endif is now grayed, further revealing that the customized file is being indexed using the stock scanner discovery. Again, there's no reason that should be happening. Customize a third file and the problem grows. Now you'll see three invocations of scanner discovery, with two of them being based on the stock profile.
I've debugged this a bit and it appears the problem is in CfgScannerConfigInfoFactory2.CfgInfo.createMap(), which for lack of finding an exact match for the customized file in 'configMap', effectively adds a new element to the map that uses the default scanner profile for the compiler tool. Set a breakpoint at the call to info = container.createInfo(baseContext, id) at line 195.
Created attachment 201110 [details] Diff file referenced in reproducibility steps
Created attachment 201286 [details] Proposed fix I've come up with this fix, but keep in mind this is the first time I've ever looked at Scanner Discovery code. So, this really needs some close scrutiny from someone more knowledgeable in that code. Unless I'm way off, there's two problems at hand. One is that the attempt to match the file to a discovery profile fails because a customized file doesn't inherently have a storageModule/scannerConfigBuildInfo element in the project description (.cproject), and the fallback position is to use a default profile. This fallback is obviously not right; we should be using the profile selected by the user for the particular configuration and language (i.e., tool + inputType). The second problem is that we create a temporary mapping, but effectively make it permanent by adding it to the project description (more specifically, by adding it to the IScannerConfigBuilderInfo2Set and leaving it there). Its permanency isn't so much the issue as is the fact that it becomes stagnant once the user selects a different profile. Only the folderInfo element (representing "/") in the .cproject is updated when the user changes the profile selection. So, prior to this fix if you customize a file, then change the Scanner profile selection, you can see in the .cproject that the custom file remains associated with the default scanner profile instead of the newly selected one. Furthermore, there's no way to remove that stale association other than by removing the element from the .cproject with a text file.
Andew, please review
Validation of the fix must include resetting the scanner profile selection after a file has been customized and the profile changed to the custom one. This will expose the second bug I referenced in comment # 3
(In reply to comment #3) > Created attachment 201286 [details] > Proposed fix I cannot apply the patch. Commit 98fa6c5..30760cf referenced in the patch is missing in CDT repository. Do you know a way of applying patches like that?
Created attachment 201500 [details] Proposed fix New patch file created with 'git diff origin/master..master'
(In reply to comment #3) > Created attachment 201286 [details] > Proposed fix > I've come up with this fix, but keep in mind this is the first time I've ever > looked at Scanner Discovery code. So, this really needs some close scrutiny from > someone more knowledgeable in that code. I have looked at his code on a few occasions but the details fade a while ago. I am afraid that is you who knows it best at the moment. What I can do just to take a look at the patch and see if it sets any alarm. > Unless I'm way off, there's two problems at hand. One is that the attempt to > match the file to a discovery profile fails because a customized file doesn't > inherently have a storageModule/scannerConfigBuildInfo element in the project > description (.cproject), and the fallback position is to use a default profile. > This fallback is obviously not right; we should be using the profile selected by > the user for the particular configuration and language (i.e., tool + inputType). > The second problem is that we create a temporary mapping, but effectively make > it permanent by adding it to the project description (more specifically, by > adding it to the IScannerConfigBuilderInfo2Set and leaving it there). Its > permanency isn't so much the issue as is the fact that it becomes stagnant once > the user selects a different profile. Only the folderInfo element (representing > "/") in the .cproject is updated when the user changes the profile selection. > So, prior to this fix if you customize a file, then change the Scanner profile > selection, you can see in the .cproject that the custom file remains associated > with the default scanner profile instead of the newly selected one. Furthermore, > there's no way to remove that stale association other than by removing the > element from the .cproject with a text file. That kind of make sense while you explain it. Just one remark - from your comments in the code, map entry is added to the container only to obtain the "info". Is it possible to create the "info" not adding it in there, that way it won't be necessary to remove it? However I have no advice how to do that, feel free to ignore that remark as rant. So flagging the review with "+".
(In reply to comment #8) > That kind of make sense while you explain it. Just one remark - from your > comments in the code, map entry is added to the container only to obtain the > "info". Is it possible to create the "info" not adding it in there, that way it > won't be necessary to remove it? However I have no advice how to do that, feel > free to ignore that remark as rant. So flagging the review with "+". The only way I see is to add a method to IScannerConfigBuilderInfo2Set to create the info but not 'record' it. In theory, this would be a little cleaner than asking it to create it and then asking it to forget it, but changing the interface to slightly optimize this single use case doesn't seem worth it. Thanks for the review.
Fix committed/pushed.
*** cdt git genie on behalf of John Cortell *** Bug 354194 - Custom scanner discovery selection is not honored if file has any custom properties [*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=e51889143728a4d3b80cee368a6784a182584024
*** cdt git genie on behalf of John Cortell *** Bug 354194 - My previous fix accidentally prevented the creation of the config info map entry for the project root folder, thus scanner discovery preferences were no longer being persisted. This adjustment adds an exeption for that rcinfo. [*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=dabef579a2a28361bbb6278a5c71a2afc25b1d0e