Community
Participate
Working Groups
I am working on a new feature to add a new type of source lookup container that is used to configure GDB's "set substitute-path from to" feature (see https://sourceware.org/gdb/onlinedocs/gdb/Source-Path.html#set%20substitute%2dpath). When used, this delegates to GDB calculating the source paths to use and leaves very little for CDT to have to take care of. With the correct "set substitute-path" set, GDB simply reports the filename field with the resolved path names meaning that CDT can do everything simply. This solution has been created to solve a key customer problem. Sharing pre-built projects between machines or relocating them on the same machine. Unfortunately there are problems with the current solutions is CDT. 1- Rebuilding the project. - Not always an option, sometimes specifically undesirable 2- Use Mapping Source Containers. - Mapping containers translate paths for setting breakpoints back to GDB, and then GDB rejects them. i.e. You typically have a map of /build/src -> /now/src because GDB is reporting a source file as /build/src/hello.c. The mapping container (the one auto-inserted by the CSourceNotFoundEditor) then converts the path to /now/src/hello.c and displays it in an editor. However, if you then insert a breakpoint in hello.c, the source container "untranslates" the path back to /build/src/hello.c for the -break-insert. At this point GDB rejects it as unknown path. I plan to submit a first pass at my solution to gerrit soon. I am very much interested in feedback and review to understand what I may have overlooked or missed. In particular I am concerned I have overlooked something regarding why CDT has not already used "set substitute-path".
New Gerrit change created: https://git.eclipse.org/r/52068 WARNING: this patchset contains 1482 new lines of code and may require a Contribution Questionnaire (CQ) if the author is not a committer on the project. Please see:https://wiki.eclipse.org/Project_Management_Infrastructure/Creating_A_Contribution_Questionnaire
Things todo: 1) Bug 202375 needs to be fixed for containers to be editable at runtime. The following needs to be done on a change: a) new set substitute-paths need to be done (and perhaps unsets too) b) breakpoints need to be reinserted (GDB may have rejected the path before set substitute-path was done) c) source path needs to re-resolve by getting frame info again from GDB 2) The CSourceNotFoundEditor needs to create (optionally, per workspace, per launch?) the new type of container. 3) The container entries currently use IPath for getBackendPath(), but that does not work for two reasons: a) Does not handle Windows <-> non-Windows path mappings. i.e. compile on Windows host, run on Linux host or vice versa b) IPath's canonicalize the names, the names must match the paths as created by GCC, i.e. as GDB returns them before set substitute-path gets involved. e.g. if the compile time path is /path/to/project/Debug/../src/Hello.c and the current path is /path/to/elsewhere/src/Hello.c then the substitution needs to be /path/to/project/Debug/../src -> /path/to/elsewhere/src as the canonicolized path of /path/to/project/src -> /path/to/elsewhere/src won't work. To solve this I need to make SourceSubstitutePathEntrySourceContainer and SourceSubstitutePathSourceContainer not extend MapEntrySourceContainer and MappingSourceContainer, respectively. I had initially had them extend the existing classes to reuse the UI elements easily (i.e. MappingSourceContainerDialog), but now I have to refactor that. 4) Some of the class names are pretty long at the moment, and not usefully so. 5) Add to Help 6) Add N&N I will be working on all of the above, thanks to funding from Renesas (http://www.renesas.eu/). I am looking forward to any feedback on my initial work. To try this out create and compile a project in one location and then move it to another on your disk. Create a new path substitution from the old project folder to the new project folder and launch the debug session. Additionally, there are tests of the functionality, see SourceLookupTest_<gdb version>.java. Starting with SourceLookupTest#sourceSubstitute() may be a good starting point for seeing the backend changes.
Not a TODO for this bug, but something that is the end goal: Following this bug being resolved, the next step (which will certainly be a new bug) is to add in the heuristics to set up source path mappings automatically. I have considered whether we can query the debugger at the point we launch a process to find out files being debugged, or to allow the DsfSourceDisplayAdapter/CSourceNotFoundEditor to have a guess what the mapping is. The problem of doing the mapping late is that breakpoints may fail to be installed if they are inserted before all the source is resolved.
New Gerrit change created: https://git.eclipse.org/r/63266
Bug 386675 will be fixed as part of this fix as "MapEntrySourceContainer.createPath" will not be used any more as I am working on changing API for backend in MapEntrySourceContainer to be stored as a String. The UNC paths is just one example of the problem, the direction of slashes and canonicalization of names is also relevant.
New Gerrit change created: https://git.eclipse.org/r/63295
Gerrit change 63295/3 rolls up the required changes for this bug. The SourceLookupTest automatically tests the various cases as part of the DSF test suite. To recap, source mapping as it is without the change fails to work in a number of circumstances. In particular, it does not work well with cross platform work (e.g. build on Windows, run/debug on Linux) and falls over in a number of places with relocated files when the compilation path was relative. There are two parts to mapping working correctly: 1) CDT is able to find, open and draw IP on a file reported by GDB. i.e. resolve a stack frame to file in an editor. (This means converting from compilation path -> local path) 2) CDT is able to take a user action in an open file and translate that back to GDB. e.g. Insert a breakpoint or Run to line. (This means converting from local path -> compilation path) This change takes advantage of gdb's set substitute-path feature (https://sourceware.org/gdb/onlinedocs/gdb/Source-Path.html#set%20substitute%2dpath) to change how source mapping is done. Here is some information on tests run: SourceLookupTest on Ubuntu Linux GCC 4.8.4 with every version of GDB from 6.6 to 7.10 (the GDB 7.10 version runs as part of the cdt-verify job) SourceLookupTest on MinGW GCC 4.8.1 with GDB version 7.6.1 Manually Compile and Run on Cygwin with GCC 4.9.3 and GDB 7.8 Manually Compile on MinGW with ARM GCC 4.8.4 and run on Linux with QEMU and GDB 7.8.0. Here are some test matrix results: Test Name [1] Current State With 63295/3 Linux AC Pass Pass AN Fail[2] Pass RC Pass Pass RN Fail[2] Pass MinGW AC Pass Pass AN Fail[2] Pass RC Pass Pass RN Fail[2] Pass Cygwin AC Pass Pass AN Fail[2] Pass RC Pass Pass RN Fail[2] Pass UNCC Fail[2] Pass with Workaround[3] UNCN Fail[2] Pass with Workaround[3] MinGW ARM compile - Linux run AC/ Pass Pass AN/ Fail[2] Pass RC/ Fail[2] Pass RN/ Fail[2] Pass UNCC Fail[2] Pass UNCN Fail[2] Pass AC\ Pass Pass AN\ Fail[2] Pass RC\ Fail[2] Pass RN\ Fail[2] Pass Notes: [1] Letter codes for tests: AC - Compilation path is absolute and canonical (no ..) e.g. gcc -o elf /path/to/file.c AN - Compilation path is absolute and non-canonical (contains ..) e.g. gcc -o elf /path/other/../to/file.c RC - Compilation path is relative and canonical (no ..) e.g. gcc -o elf to/file.c RN - Compilation path is relative and non-canonical (contains ..) e.g. gcc -o elf ../to/file.c UNCC - Compilation path is UNC and canonical e.g. gcc -o elf \\machine\share\path\to\file.c UNCN - Compilation path is UNC and non-canonical e.g. gcc -o elf \\machine\share\path\other\..\to\file.c / suffix - Compilation path has forward slashes e.g. gcc -o elf c:/path/to/file.c \ suffix - Compilation path has back slashes e.g. gcc -o elf c:\path\to\file.c Pass - means everything just works Fail - means at least one thing does not work and there is no straightforward workaround Pass with Workaround - means see note [2] Source file can be located, but breakpoints can not be inserted. Features such as Run To Line do not work. [3] With Cygwin if compilation path is UNC, breakpoints cannot be inserted on the UNC name (like [2]), however if at debug time the UNC path is mounted by OS then breakpoints do work.
I have updated the "Blocks" list for all the bugs that are fixed (or most likely fixed based on the provided information) by Gerrit change 63295/3. Also, while we are in the source lookup area, in the effort to ditch old bugs, Bug 159952 and Bug 159953 are feature requests that are almost 10 years old so while brains are in that area, perhaps expand on what those requests are if they are still relevant, or ditch them if no longer relevant.
Gerrit change https://git.eclipse.org/r/63266 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=cedcf7655b1f3a2c305de5ae08cc697f60fdc39a
(In reply to Eclipse Genie from comment #9) > Gerrit change https://git.eclipse.org/r/63266 was merged to [master]. > Commit: > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > ?id=cedcf7655b1f3a2c305de5ae08cc697f60fdc39a I have seen that there are a few floating around failures because of this change. They all look like these: from https://hudson.eclipse.org/cdt/job/cdt-verify/4142/testReport/org.eclipse.cdt.tests.dsf.gdb.tests.tests_7_10/GDBMultiNonStopRunControlTest_7_10/testStateProcessTwoThreadsStoppedAndRunning/: java.lang.AssertionError: Something has gone wrong, there is an unterminated launch from a previous test! at org.junit.Assert.fail(Assert.java:88) at org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase.removeTeminatedLaunchesBeforeTest(BaseTestCase.java:221) at org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase.doBeforeTest(BaseTestCase.java:236) at org.eclipse.cdt.tests.dsf.gdb.tests.tests_7_0.GDBMultiNonStopRunControlTest_7_0.doBeforeTest(GDBMultiNonStopRunControlTest_7_0.java:66) from https://hudson.eclipse.org/cdt/job/cdt-master/737/testReport/junit/org.eclipse.cdt.tests.dsf.gdb.tests.tests_7_10/MIExpressionsTest_7_10/testWriteVariable/ org.eclipse.debug.core.DebugException: Failed to delete launch configuration. at org.eclipse.debug.internal.core.LaunchConfiguration.delete(LaunchConfiguration.java:284) at org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase.removeTeminatedLaunchesBeforeTest(BaseTestCase.java:230) at org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase.doBeforeTest(BaseTestCase.java:236) at org.eclipse.cdt.tests.dsf.gdb.tests.MIExpressionsTest.doBeforeTest(MIExpressionsTest.java:94) I am pretty sure these failures are exposing a problem that was always there. Namely that launches are not being cleaned up properly so are remaining around after a test terminates and are not being successfully terminated, or not in time. The timeout is already massaged so a little more research is needed.
New Gerrit change created: https://git.eclipse.org/r/64609
Gerrit change https://git.eclipse.org/r/64609 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=6d5d0be628f79c28a4b11f1053c0cdbbec27100a
New Gerrit change created: https://git.eclipse.org/r/65305
(In reply to Jonah Graham from comment #10) > (In reply to Eclipse Genie from comment #9) > > Gerrit change https://git.eclipse.org/r/63266 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > > ?id=cedcf7655b1f3a2c305de5ae08cc697f60fdc39a > > I have seen that there are a few floating around failures because of this > change. They all look like these: > > from > https://hudson.eclipse.org/cdt/job/cdt-verify/4142/testReport/org.eclipse. > cdt.tests.dsf.gdb.tests.tests_7_10/GDBMultiNonStopRunControlTest_7_10/ > testStateProcessTwoThreadsStoppedAndRunning/: > > java.lang.AssertionError: Something has gone wrong, there is an unterminated > launch from a previous test! > at org.junit.Assert.fail(Assert.java:88) > at > org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase. > removeTeminatedLaunchesBeforeTest(BaseTestCase.java:221) > at > org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase. > doBeforeTest(BaseTestCase.java:236) > at > org.eclipse.cdt.tests.dsf.gdb.tests.tests_7_0. > GDBMultiNonStopRunControlTest_7_0. > doBeforeTest(GDBMultiNonStopRunControlTest_7_0.java:66) https://git.eclipse.org/r/65305 is to fix the above exception.
(In reply to Jonah Graham from comment #10) > from > https://hudson.eclipse.org/cdt/job/cdt-master/737/testReport/junit/org. > eclipse.cdt.tests.dsf.gdb.tests.tests_7_10/MIExpressionsTest_7_10/ > testWriteVariable/ > > org.eclipse.debug.core.DebugException: Failed to delete launch configuration. > at > org.eclipse.debug.internal.core.LaunchConfiguration. > delete(LaunchConfiguration.java:284) > at > org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase. > removeTeminatedLaunchesBeforeTest(BaseTestCase.java:230) > at > org.eclipse.cdt.tests.dsf.gdb.framework.BaseTestCase. > doBeforeTest(BaseTestCase.java:236) > at > org.eclipse.cdt.tests.dsf.gdb.tests.MIExpressionsTest. > doBeforeTest(MIExpressionsTest.java:94) This case seems impossible for me to reproduce. I have only seen the above exception one time (in build cdt-master/737) and I cannot see how it should be possible. The exception comes from LaunchConfiguration.delete. In there the .launch file is deleted and is immediately checked for existence. If it exists the above exception is raised. if (store != null) { store.delete(EFS.NONE, null); if ((store.fetchInfo().exists())) { throw new DebugException( new Status(IStatus.ERROR, DebugPlugin.getUniqueIdentifier(), DebugException.REQUEST_FAILED, DebugCoreMessages.LaunchConfiguration_Failed_to_delete_launch_configuration__1, null) ); } } I am not planning on investigating further unless a further build fails. If it does, I suspect the error is over in platform debug?
Gerrit change https://git.eclipse.org/r/65305 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=84d92dce1f2c07dc6a7b9f6dbd2743635d45ca77
Gerrit change https://git.eclipse.org/r/52068 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=628389071558c43c52b666995e1eaa5c4aa67a8f
(In reply to Eclipse Genie from comment #17) > Gerrit change https://git.eclipse.org/r/52068 was merged to [master]. > Commit: > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > ?id=628389071558c43c52b666995e1eaa5c4aa67a8f Took 8 months to the day but it's in. We won't make it a habit to take this long. Thanks Jonah for your hard work. Can you put a note in the N&N https://wiki.eclipse.org/CDT/User/NewIn90 Shall we close this bug?
New Gerrit change created: https://git.eclipse.org/r/68641
(In reply to Marc Khouzam from comment #18) > Took 8 months to the day but it's in. We won't make it a habit to take this > long. Thanks Jonah for your hard work. Thanks for your hard work on the review, it is a large and sticky area of the code to get into and with your thorough review I am much more confident that we have a great solution here. > Can you put a note in the N&N > https://wiki.eclipse.org/CDT/User/NewIn90 I will do. > Shall we close this bug? Not yet :-( There is still another part of this bug https://git.eclipse.org/r/#/c/63295/ That part is needed to resolve two key issues: 1- Building on one platform, debugging on another - i.e. forward / vs backwards \ slashes, but device name C:, D: 2- Handling non-canonical paths properly in compile path, e.g. when gcc was run originally like gcc -o a.o ../src/a.c The second one is very important as that is how CDT Managed Make projects compile their sources, so until we can apply that change path substitutions by GDB won't work for the default CDT project type! >
(In reply to Jonah Graham from comment #20) > 1- Building on one platform, debugging on another - i.e. forward / vs > backwards \ slashes, but device name C:, D: Not sure where that but came from, it was supposed to read: 1- Building on one platform, debugging on another - i.e. forward / vs backwards \ slashes, and device names C:, D:
(In reply to Jonah Graham from comment #20) > The second one is very important as that is how CDT Managed Make projects > compile their sources, so until we can apply that change path substitutions > by GDB won't work for the default CDT project type! I'm glad you have a solution for this one. I was only able to get the path mapping to work with an absolute path and not with a project built in CDT. Now I understand why.
debug/org.eclipse.cdt.debug.ui.tests/core/org/eclipse/cdt/debug/core/tests/MapEntrySourceContainerTests.java was removed as part of the CDI cleanup. Why did we do that?
Gerrit change https://git.eclipse.org/r/68641 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=e977bb49533a75a44ac3386bd6973cb90b6d1401
(In reply to Marc Khouzam from comment #23) > debug/org.eclipse.cdt.debug.ui.tests/core/org/eclipse/cdt/debug/core/tests/ > MapEntrySourceContainerTests.java > > was removed as part of the CDI cleanup. Why did we do that? It should not have been. The rest of the tests in plug-in were CDI specific. At the time I removed them I had a version of Source Lookup that removed MapEntrySourceContainer.createPath() so I suspect I got confused. I will restore the test, although it is of little value. I will restore it however to org.eclipse.cdt.debug.core.tests (it was in org.eclipse.cdt.debug.ui.tests) because it is a core thing being tested. (Strangely org.eclipse.cdt.debug.core.tests is an empty plug-in, just about.html and .project file.)
(In reply to Jonah Graham from comment #25) > (In reply to Marc Khouzam from comment #23) > > debug/org.eclipse.cdt.debug.ui.tests/core/org/eclipse/cdt/debug/core/tests/ > > MapEntrySourceContainerTests.java > > > > was removed as part of the CDI cleanup. Why did we do that? > > It should not have been. The rest of the tests in plug-in were CDI specific. > At the time I removed them I had a version of Source Lookup that removed > MapEntrySourceContainer.createPath() so I suspect I got confused. I will > restore the test, although it is of little value. > > I will restore it however to org.eclipse.cdt.debug.core.tests (it was in > org.eclipse.cdt.debug.ui.tests) because it is a core thing being tested. > (Strangely org.eclipse.cdt.debug.core.tests is an empty plug-in, just > about.html and .project file.) When I do restore it, I will move MapEntrySourceContainerTests (from https://git.eclipse.org/r/#/c/63282/11/debug/org.eclipse.cdt.debug.ui.tests/core/org/eclipse/cdt/debug/core/tests/MapEntrySourceContainerTests.java) to it also, as that is where it belongs.
Gerrit change https://git.eclipse.org/r/63295 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=a56abb4783d3029f54a523fad248d8dceec4497d
(In reply to Eclipse Genie from comment #27) > Gerrit change https://git.eclipse.org/r/63295 was merged to [master]. > Commit: > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > ?id=a56abb4783d3029f54a523fad248d8dceec4497d I believe we are done! Nice job Jonah.
(In reply to Marc Khouzam from comment #28) > (In reply to Eclipse Genie from comment #27) > > Gerrit change https://git.eclipse.org/r/63295 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/ > > ?id=a56abb4783d3029f54a523fad248d8dceec4497d > > I believe we are done! Nice job Jonah. Thank you for your thorough review Marc. I will now proceed with the non-code part of this bug: - Update N&N - Verify and close other bugs that are resolved by this fix - Restore accidentally removed test, see Comment 26 - Restore test comments removed during test refactor, see Gerit[1] - Update documentation/online help - Create bug for next part of this feature, see Comment 3 Thanks again! Jonah [1] https://git.eclipse.org/r/#/c/68611/15/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_6/SourceLookupTest_7_6.java
New Gerrit change created: https://git.eclipse.org/r/70234
Gerrit change https://git.eclipse.org/r/70234 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=a925a51cf49a79eb00441b988808bad10d51fc77
New Gerrit change created: https://git.eclipse.org/r/70475
New Gerrit change created: https://git.eclipse.org/r/70485
New Gerrit change created: https://git.eclipse.org/r/70486
(In reply to Jonah Graham from comment #29) > I will now proceed with the non-code part of this bug: > > - Update N&N Done: https://wiki.eclipse.org/CDT/User/NewIn90#Improved_Source_Lookup_Path_Mapping > - Verify and close other bugs that are resolved by this fix Done, Bugs 129349 159954 225805 235181 327167 377387 386675 477057 are all fixed by this change. > - Restore accidentally removed test, see Comment 26 On further review this was not an accidentally removed test. It should have been removed with more of a comment perhaps. It was actually a failing test that was demonstrating a bug in source lookup and as such was not run as part of the build. See Bug 386675. > - Restore test comments removed during test refactor, see Gerit[1] Done: https://git.eclipse.org/r/#/c/70475/ > - Update documentation/online help Done: https://git.eclipse.org/r/#/c/70485/ > - Create bug for next part of this feature, see Comment 3 Done: Bug 491514 (Note the gerrits are waiting for a clean build, but they don't change code and will be submitted once they get their +1s from hudson)
Gerrit change https://git.eclipse.org/r/70486 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=64da973dcf867afdf81f2255f2923646d864ae3c
Gerrit change https://git.eclipse.org/r/70475 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=6f249f12c1aeaea0092c424dda0450fef159ffe8
Gerrit change https://git.eclipse.org/r/70485 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=93155ecebad60f1218ac0537c333863ca94f430c
