eclipse-cdt-cloud / cdt-gdb-adapter

CDT GDB Debug Adapter
Eclipse Public License 2.0
28 stars 40 forks source link

Fix issue related to cannot set breakpoints #201

Closed thamho0902 closed 1 year ago

eclipse-cdt-bot commented 2 years ago

Can one of the admins verify this patch?

thamho0902 commented 2 years ago

Can you please describe the issue you are fixing and provide test that fails without the change and passes with it. Hi @jonahgraham The issue occurs when:

  1. Open VS code -> Open folder (project already built)
  2. Click Run and Debug -> start Debugging
  3. In project.c set breakpoint in code -> Error message "e2-server-gdb.exe has exited with code 3221225477" => Exit the program without setting breakpoint So I checked the program and found there was an error in the variable syntax and an error in handling "-exec-interrupt -all" in single core. so that's my patch for this problem Please help me check it. Thank you!
thamho0902 commented 2 years ago

Thanks @thamho0902 for the patch. It looks promising and fixes stuff that there clearly wasn't a test for. Can you please describe the issue you are fixing and provide test that fails without the change and passes with it.

Also you need to sort out the ECA - @quyettrinhrvc or @QuocDoBV should be able to help as they both had to resolve this recently.

Hi @jonahgraham What do you mean I need more integration tests to check this problem?

jonahgraham commented 2 years ago

What do you mean I need more integration tests to check this problem?

Currently when I run yarn test (without your patch applied) all tests pass. So please provide a test that demonstrates your fix. The test should fail without your patch applied and pass once it is applied.

jonahgraham commented 2 years ago

There are existing tests that can be adapter/extended. Please let me know how I can help.

These tests are useful places to start:

IIUC the bug is when a breakpoint is inserted while the program is running. There is no test for that as of now. 

If I were writing this test, and it can be reproduced in a single threaded program, I would add a new test to breakpoints.spec.ts. To prevent having to modify existing tests, I would modify count.c to turn its existing loop into an infinite loop, but maintain the features of the file so that existing tests still work, something like:

int main() {
    int count = 0, another = 0;
    while (1) {
        count ++; 
        // line with no code
        another ++;
    }    
    return 0;
}

Add a new test (it(...)) based on one of the existing simple ones, and then resume the debugging session, and then insert a breakpoint on count++ line (line 4) and make sure the program stops there.


To do this I would start with one of the existing tests up until the point it is stopped:

        let response = await dc.setBreakpointsRequest({
            source: {
                name: 'count.c',
                path: path.join(testProgramsDir, 'count.c'),
            },
            breakpoints: [
                {
                    column: 1,
                    line: 2,
                },
            ],
        });
        expect(response.body.breakpoints.length).to.eq(1);

        await dc.configurationDoneRequest();
        await dc.waitForEvent('stopped');

Once it is stopped, continue the target. You can base this on how continueRequest is done in debugClient.ts something like:

        const scope = await getScopes(dc);
        await dc.continueRequest({ threadId: scope.thread.id });

insert the new breakpoint at line 4 (just as above, but for line 4), something like this should work:

        response = await dc.setBreakpointsRequest({
            source: {
                name: 'count.c',
                path: path.join(testProgramsDir, 'count.c'),
            },
            breakpoints: [
                {
                    column: 1,
                    line: 4,
                },
            ],
        });
        expect(response.body.breakpoints.length).to.eq(1);

Then wait to make sure the target stops at the expected breakpoint, functionBreakpoints.spec.ts does this a bit so you can use that as a model. Probably something like this:

        await dc.assertStoppedLocation('breakpoint', { line: 4 });

Note as this issues only affects non-stop mode, I have extended the testsuite to always run all tests in both non-stop and all-stop mode. See PR206 for more info. To run just your new test you can use the it.only syntax and do yarn test:integration-gdb-non-stop from the command line, or see the tips for other options..

jonahgraham commented 2 years ago

One additional note, non-stop does not work on Windows when doing native debugging. Therefore writing and running tests there can be a problem.

thamho0902 commented 2 years ago

Dear @jonahgraham, I added integration tests based on your suggestion to check the problem. I have also run them on my environment. Here is the log file with the old and new code. LogReport.zip Could you please help me review it? Thanks

thamho0902 commented 2 years ago

Hi @jonahgraham Please help me check it again. This is the log file that I run in my environment. LogFile_AddBreakpoint.zip LogFile_setFunctionBreakpoint.zip Thanks

jonahgraham commented 2 years ago

Note that in reviewing this change I realized one important combination was missing from the test matrix. See #208 - please make sure you merge latest main to make sure that remote non-stop debugging tests pass.

thamho0902 commented 2 years ago

I see numerous test failures when running with these changes. e.g. in remote all-stop I see:

[12:23:06.801 UTC] From client: setFunctionBreakpoints({"breakpoints":[{"name":"sub"}]})
[12:23:06.801 UTC] GDB command: 12 -thread-info
[12:23:06.801 UTC] GDB result: 12 error,msg="Cannot execute this command while the target is running.\nUse the \"interrupt\" command to stop the target\nand then try again."

Does yarn test run all the tests in all the configurations for you and do they all pass? If so, please tell me what versions of gdb/gcc/gdbserver you are using so we can track down the differences.

Dear @jonahgraham , Honestly, I have just run test cases related to setting breakpoint by command yarn test:integration-gdb-non-stop. The result as I attached before. I have just run again test cases with latest code(not include my changes), then there are a lots of cases failed (about 3 times) test-reports.zip I'm concerning that weather failure you mentioned have related to my changes or not. This is my environment. Screenshot from 2022-09-14 22-46-03 So I have some concerns:

jonahgraham commented 2 years ago

The build machine and my local machine successfully runs the current main branch with no test errors. See the last build's test result which was run on commit 95d05edcaced3e04836cbe5e0f0db0d17b9f9bef

When I run your new tests I get errors. e.g. set it.only on your new functionBreakpoints test and do yarn test I get the failure outlined in https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/201#pullrequestreview-1107436724 here is my full output:

$ yarn test:integration && yarn test:pty && yarn test:integration-run-in-terminal && yarn test:integration-remote-target && yarn test:integration-remote-target-run-in-terminal && yarn test:integration-gdb-async-off && yarn test:integration-gdb-async-off-remote-target && yarn test:integration-gdb-non-stop && yarn test:integration-gdb-non-stop-remote-target
$ cross-env JUNIT_REPORT_PATH=test-reports/integration.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --reporter mocha-jenkins-reporter dist/integration-tests/*.spec.js

  function breakpoints
      ✔ can set and hit the sub function breakpoint while program is running: 222ms

  Suite duration: 0.45 s, Tests: 1

  1 passing (546ms)

$ cross-env JUNIT_REPORT_PATH=test-reports/native.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --reporter mocha-jenkins-reporter dist/native/*.spec.js

  pty creation
      ✔ should be able to open a ptmx/pts pair: 70ms

  Suite duration: 0.076 s, Tests: 1

  1 passing (77ms)

22 EIO: i/o error, read
$ cross-env JUNIT_REPORT_PATH=test-reports/integration-run-in-terminal.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --run-in-terminal --reporter mocha-jenkins-reporter dist/integration-tests/*.spec.js

  function breakpoints
      ✔ can set and hit the sub function breakpoint while program is running: 215ms

  Suite duration: 0.5 s, Tests: 1

  1 passing (511ms)

$ cross-env JUNIT_REPORT_PATH=test-reports/integration-remote-target.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --test-remote --timeout 5000 --reporter mocha-jenkins-reporter dist/integration-tests/*.spec.js

  function breakpoints
      1) can set and hit the sub function breakpoint while program is running

  Suite duration: 5.272 s, Tests: 1

  0 passing (5s)
  1 failing

  1) function breakpoints
       can set and hit the sub function breakpoint while program is running:
     Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/scratch/debug/git/cdt-gdb-adapter/dist/integration-tests/functionBreakpoints.spec.js)
      at listOnTimeout (internal/timers.js:557:17)
      at processTimers (internal/timers.js:500:7)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
jonahgraham commented 2 years ago

Note that in reviewing this change I realized one important combination was missing from the test matrix. See #208 - please make sure you merge latest main to make sure that remote non-stop debugging tests pass.

Please merge in main again - there was a typo in my last commit. Current main HEAD (6766c8127add7bd4849f22f9da01243dda4fce8c) has all tests passing on build machine and my local machine.

jonahgraham commented 2 years ago

To explicitly answer your questions:

Do you mean the failed cases cause by my change in GDBDebugSession.ts and thread.ts or by changing in integration tests (include source .C)?

If I disable your two new tests, merge in current main to your branch, all the tests pass.

How do you think about integration tests which I added. Do I need to change anything?

The tests look ok, it is the code that is the problem - you can't obtain thread list from running remote target in all-stop (or at least I can't). If you are able to, please let me know more about your environment so we can track down whether we need to fix it.

Could you run all integration tests in your environment? I just want to confirm if I have correct environment or not.

This part is answered in https://github.com/eclipse-cdt-cloud/cdt-gdb-adapter/pull/201#issuecomment-1247009631

jonahgraham commented 2 years ago

I am using the same version of gdb, gdbserver as you, but using gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0. The build machine uses gdb/gdbserver (Ubuntu 8.1.1-0ubuntu1) 8.1.1 and gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

thamho0902 commented 2 years ago

Hi @jonahgraham With my changes, user can set breakpoint in all cases without GDB server set with -gdb-set non-stop off and -gdb-set mi-async on

thamho0902 commented 2 years ago

Dear @jonahgraham, Currently with my changes, we can add breakpoints in most cases but GDB server is set -gdb-set non-stop off and -gdb-set mi-async off. I thinks this cases is special cases. we cannot send any command when target is running. So I thinks in this case we cannot set breakpoints when target is running. I thinks it is limitation. This is the logfile:

[08:38:03.951 UTC] GDB result: 0 done
[08:38:03.952 UTC] GDB command: 1 -gdb-set mi-async off
[08:38:03.953 UTC] GDB result: 1 done
[08:38:03.954 UTC] GDB command: 2 -file-exec-and-symbols "/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions"
[08:38:03.957 UTC] GDB result: 2 done
[08:38:03.958 UTC] GDB command: 3 -enable-pretty-printing
[08:38:03.958 UTC] GDB result: 3 done
[08:38:03.959 UTC] To client: {"seq":0,"type":"event","event":"initialized"}
[08:38:03.960 UTC] To client: {"seq":0,"type":"response","request_seq":2,"command":"launch","success":true}
[08:38:03.961 UTC] From client: setFunctionBreakpoints({"breakpoints":[{"name":"main"}]})
[08:38:03.963 UTC] GDB command: 4 -break-list
[08:38:03.964 UTC] GDB result: 4 done,BreakpointTable={nr_rows="0",nr_cols="6",hdr=[{width="7",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="10",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[]}
[08:38:03.967 UTC] GDB command: 5 -break-insert --function main
[08:38:03.968 UTC] GDB result: 5 done,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000001156",func="main",file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",thread-groups=["i1"],times="0",original-location="-function main"}
[08:38:03.969 UTC] To client: {"seq":0,"type":"response","request_seq":3,"command":"setFunctionBreakpoints","success":true,"body":{"breakpoints":[{"id":1,"verified":true}]}}
[08:38:03.971 UTC] From client: configurationDone(undefined)
[08:38:03.972 UTC] GDB command: 6 -exec-run
[08:38:03.973 UTC] GDB notify async: thread-group-started,id="i1",pid="32467"
[08:38:03.973 UTC] GDB notify async: thread-created,id="1",group-id="i1"
[08:38:03.984 UTC] GDB notify async: breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000555555555156",func="main",file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",thread-groups=["i1"],times="0",original-location="-function main"}
[08:38:03.986 UTC] GDB notify async: library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1",ranges=[{from="0x00007ffff7fd0100",to="0x00007ffff7ff2684"}]
[08:38:04.033 UTC] GDB result: 6 running
[08:38:04.034 UTC] GDB exec async: running,thread-id="all"
[08:38:04.035 UTC] To client: {"seq":0,"type":"response","request_seq":4,"command":"configurationDone","success":true}
[08:38:04.053 UTC] GDB notify async: library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="0",thread-group="i1",ranges=[{from="0x00007ffff7de6630",to="0x00007ffff7f5b27d"}]
[08:38:04.818 UTC] GDB notify async: breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000555555555156",func="main",file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",thread-groups=["i1"],times="1",original-location="-function main"}
[08:38:04.820 UTC] To client: {"seq":0,"type":"event","event":"output","body":{"category":"stdout","output":"\n"}}
[08:38:04.821 UTC] To client: {"seq":0,"type":"event","event":"output","body":{"category":"stdout","output":"Breakpoint 1, main () at functions.c:14\n"}}
[08:38:04.822 UTC] To client: {"seq":0,"type":"event","event":"output","body":{"category":"stdout","output":"14\t{   staticfunc1(); // make the line of code the same as opening brace to account for different gdb/gcc combinations\n"}}
[08:38:04.823 UTC] GDB exec async: stopped,reason="breakpoint-hit",disp="keep",bkptno="1",frame={addr="0x0000555555555156",func="main",args=[],file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",arch="i386:x86-64"},thread-id="1",stopped-threads="all",core="0"
[08:38:04.825 UTC] To client: {"seq":0,"type":"event","event":"stopped","body":{"reason":"function breakpoint","allThreadsStopped":true,"threadId":1}}
[08:38:04.830 UTC] From client: threads(undefined)
[08:38:04.831 UTC] GDB command: 7 -thread-info
[08:38:04.834 UTC] GDB result: 7 done,threads=[{id="1",target-id="process 32467",name="functions",frame={level="0",addr="0x0000555555555156",func="main",args=[],file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",arch="i386:x86-64"},state="stopped",core="0"}],current-thread-id="1"
[08:38:04.838 UTC] To client: {"seq":0,"type":"response","request_seq":5,"command":"threads","success":true,"body":{"threads":[{"id":1,"name":"functions","running":false}]}}
[08:38:04.841 UTC] From client: stackTrace({"threadId":1})
[08:38:04.841 UTC] GDB command: 8 -stack-info-depth --thread 1 100
[08:38:04.842 UTC] GDB result: 8 done,depth="1"
[08:38:04.843 UTC] GDB command: 9 -stack-list-frames --thread 1 0 0
[08:38:04.844 UTC] GDB result: 9 done,stack=[frame={level="0",addr="0x0000555555555156",func="main",file="functions.c",fullname="/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c",line="14",arch="i386:x86-64"}]
[08:38:04.845 UTC] To client: {"seq":0,"type":"response","request_seq":6,"command":"stackTrace","success":true,"body":{"stackFrames":[{"id":1000,"source":{"name":"functions.c","path":"/home/quoc/Data/VScode/Source/ThamHo/cdt-gdb-adapter/src/integration-tests/test-programs/functions.c","sourceReference":0},"line":14,"column":0,"name":"main","instructionPointerReference":"0x0000555555555156"}],"totalFrames":1}}
[08:38:04.848 UTC] From client: scopes({"frameId":1000})
[08:38:04.848 UTC] To client: {"seq":0,"type":"response","request_seq":7,"command":"scopes","success":true,"body":{"scopes":[{"name":"Local","variablesReference":1000,"expensive":false},{"name":"Registers","variablesReference":1001,"expensive":true}]}}
[08:38:04.849 UTC] From client: continue({"threadId":1})
[08:38:04.850 UTC] GDB command: 10 -exec-continue --thread 1
[08:38:04.858 UTC] GDB result: 10 running
[08:38:04.858 UTC] GDB exec async: running,thread-id="all"
[08:38:04.858 UTC] To client: {"seq":0,"type":"response","request_seq":8,"command":"continue","success":true}
[08:38:04.859 UTC] From client: setFunctionBreakpoints({"breakpoints":[{"name":"sub"}]})
[08:38:04.860 UTC] GDB command: 11 -exec-interrupt --all
[08:38:05.974 UTC] From client: disconnect(undefined)
[08:38:05.975 UTC] GDB command: 12 -gdb-exit
jonahgraham commented 1 year ago

GitHub actions are now fully enabled and seem to be working. There is a green tick on the last commit and in the details you can see that both Windows and Ubuntu tests all passed (or were appropriately skipped).

@thamho0902 I look forward to your review so that we can get this merged or fix up any additional issues that are identified.

thamho0902 commented 1 year ago

Hi @jonahgraham Thank you very much for your support. I tested it with my Renesas board without that issue again. Please help me merge it.

jonahgraham commented 1 year ago

This change is merged and now available on npm - https://www.npmjs.com/package/cdt-gdb-adapter/v/0.0.16-next.20220919143213.36a785a.0