vscode-cmake-tools icon indicating copy to clipboard operation
vscode-cmake-tools copied to clipboard

GCC linker errors not showing up in Problems window

Open RolfNoot opened this issue 2 years ago • 27 comments

Brief Issue Summary

I am not sure whether this is an issue or expected behavior. Compiler errors are shown correctly.

Schermafbeelding 2022-11-22 om 17 17 27

CMake Tools Diagnostics

No response

Debug Log

No response

Additional Information

No response

RolfNoot avatar Nov 22 '22 16:11 RolfNoot

This would be a bug/feature request. We have a regular expression that matches compiler errors, but the linker errors don't match that pattern. Thanks for reporting it. We may not get to this immediately, but will accept a PR from the community if someone is able to address it before we do.

bobbrow avatar Nov 22 '22 17:11 bobbrow

Thanks for the quick reply. Where (in code?) can I find this regex? I might change it according my needs.

RolfNoot avatar Nov 22 '22 19:11 RolfNoot

It is here: https://github.com/microsoft/vscode-cmake-tools/blob/a00c339cef191fb9bae64922713189bf64d994b0/src/diagnostics/gcc.ts#L9

bobbrow avatar Nov 22 '22 19:11 bobbrow

Changing the regex doesn't affect the output. I changed the one in vscode-cmake-tools/src/diagnostics/msvc.ts which seems to affect the output.

How can I select the problemMatcher when starting build programmatically (not using Tasks) vscode.commands.executeCommand('cmake.build')?

RolfNoot avatar Nov 23 '22 21:11 RolfNoot

I believe that all the problem matchers are active when you build with our commands (not tasks). You can control which ones are active with the cmake.enabledOutputParsers setting.

bobbrow avatar Nov 23 '22 21:11 bobbrow

This issue also exists in MSVC builds. MSVC link errors are not matched by the problem matcher.

evakili avatar Jan 04 '23 07:01 evakili

It is because the VSCode does not provide an API to show the problems without of a file path. I meant, that usually, the linker errors does not a file paths at all.

denis-shienkov avatar Feb 03 '23 16:02 denis-shienkov

It can show problems using a 'null' path. Besides that, the file path is provided by the linker and should be matched by the regex (/Volumes/.../main.c:105 in this case).

I am currently using an additional problemmatcher for linker errors which works fine.

This should be implemented in the extension IMHO.

"problemMatcher": [
        {
            "base": "$gcc",
            "fileLocation": ["relative", "${workspaceFolder}/build" ]
        },
        {
            "owner": "linker0",
            "severity": "error",
            "fileLocation" : "absolute",
            "pattern": {
            "regexp": "((error): ld returned (-?\\d+) exit status)",
            "message": 1,
            "file" : 2
            }
        },
        {
            "owner": "linker1",
            "severity": "error",
            "fileLocation" : "absolute",
            "pattern": {
            "regexp": "(\\S*\\..{0,2}):(\\d*):\\s(undefined reference to `\\S*')",
            "file": 1,
            "line": 2,
            "message": 3
            }
        },
        {
            "owner": "linker2",
            "severity": "error",
            "fileLocation" : "absolute",
            "pattern": {
            "regexp": "((.*\\..{0,2}):(\\d+): (multiple definition of .+);.+:(.*\\..{0,2}):(\\d+): first defined here)",
            "message": 4,
            "file": 5,
            "line": 3
            }
        },
        {
            "owner": "linker3",
            "severity": "error",
            "fileLocation" : "absolute",
            "pattern": {
            "regexp": "((cannot open linker script file (.+.ld): No such file or directory))",
            "message": 1,
            "file": 3
            }
        },
        {
            "owner": "linker4",
            "severity": "error",
            "fileLocation" : "absolute",
            "pattern": {
            "regexp": "((region `\\S+' overflowed by \\d+ bytes))",
            "message": 1
            }
        }

RolfNoot avatar Feb 03 '23 20:02 RolfNoot

Besides that, the file path is provided by the linker

This is a special case that there is a path to the file. In most cases, linking errors do not contain any paths. This will not only affect GCC but also other compilers (eg MSVC, IAR, KEIL, SDCC and etc).

So, in a common case you can't extract the file property for the problem matcher... And, what's to do in such case is unknown..

I even created an issue about that:

  • https://github.com/microsoft/vscode-cpptools/issues/6643
  • https://github.com/microsoft/vscode/issues/112145

but all was unsucessfull.

denis-shienkov avatar Feb 04 '23 08:02 denis-shienkov

What you can do in such a case is use a 'null' path for the file. No file path

In my opinion, this is currently the best way to report linker errors without file reference. Not reporting linker errors is really not a good option for most users. In the future, I hope VS Code will implement a way to implement error reporting without file reference.

RolfNoot avatar Feb 04 '23 11:02 RolfNoot

Hmm.. Is it possible at all to set a null path? Because the vscode.DiagnosticCollection expects the the method set(uri: Uri, diagnostics: readonly Diagnostic[]): void use an vscode.Uri type. I'm not sure that it is possible to assign a null to the Uri (I'm not a typescript expert at all).

denis-shienkov avatar Feb 04 '23 16:02 denis-shienkov

I am assuming it's a 'null' path, an empty string or just a '/' which is a valid Uri. The screenshot above shows that it's possible at least. Not sure how the problemmatcher internally works and I am not typescript expert either.

RolfNoot avatar Feb 04 '23 17:02 RolfNoot

It is here:

https://github.com/microsoft/vscode-cmake-tools/blob/a00c339cef191fb9bae64922713189bf64d994b0/src/diagnostics/gcc.ts#L9

Maybe the regex fails because there is a [build] in front of the actual error message, e.g.: [build] C:\project_dir\user_code.c:45:23: error: CAN_BUS_0 not declared (or typename)

Following regex will match above line: ^\[build\] (.*):(\d+):(\d+):\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)

EDIT: running cmake --build . does not show the [build] thingy in front of an error message and vscode/gcc out parser DOES successful parse the output and show it under problems or within the editor:

grafik

kuch3n avatar Feb 17 '24 20:02 kuch3n

I tinkered with the gcc parser because showing the linker errors in the Problems panel is really helpful.

What I basically did is I added some more regex cases and modified the handling of the additional matches. If there is no column, 1 is set as column, if there is no severity, error is set as severity, etc.

Here are the Typescript Regex trial and errors is used, if anyone wants to add some more special cases.

Here is the modified gcc parser:

/**
 * Module for handling GCC diagnostics
 */ /** */
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.Parser = exports.REGEX = void 0;
const vscode = __webpack_require__(89496);
const util_1 = __webpack_require__(52919);

const MatchType = {
    Full: 0,
    File: 1,
    Line: 2,
    Column: 3,
    Severity: 4,
    Message: 5
};

const regexPatterns = [
    {   // path/to/file:line:column: severity: message
        regexPattern: /^(?:(.*):(\d+):(\d+):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Column, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/ld[.exe]:path/to/file:line: warning: memory region ... not declared
        regexPattern: /^(?:(?:(?:.*ld\:)|(?:.*ld\.exe\:))(.*):(\d+):)\s+(.*): (memory region .* not declared)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/ld[.exe]:path/to/file:line: syntax error
        regexPattern: /^(?:(?:(?:.*ld\:)|(?:.*ld\.exe\:))(.*):(\d+):) (syntax error)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Message], 
    },
    {   // path/to/ld.exe: severity: message
        regexPattern: /^(?:(.*ld\.exe):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/ld: severity: message
        regexPattern: /^(?:(.*ld):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/ld.exe: message
        regexPattern: /^(?:(.*ld\.exe):)\s+(.*[^:]$)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Message], 
    },
    {   // path/to/ld: message
        regexPattern: /^(?:(.*ld):)\s+(.*)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Message], 
    },
    {   // path/to/file:line: severity: message
        regexPattern: /^(?:(.*):(\d+):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/file:line: undefined reference to
        regexPattern: /^(?:(.*):(\d+):)\s+(undefined reference to .*)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Message], 
    },
    {   // path/to/file: ... section ... will not fit in region ...
        regexPattern: /^(.*): ((?:.*) .*section.* (?:.*) .*will not fit in region.*)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Message], 
    },
    {   // path/to/file:line: multiple definition of ... first defined here
        regexPattern: /^(?:(.*):(\d+):)\s+(multiple definition of .* first defined here)/, 
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Line, MatchType.Message], 
    },
    {   // path/to/cc1.exe: severity: message
        regexPattern: /^(?:(.*cc1\.exe):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Severity, MatchType.Message], 
    },
    {   // path/to/arm-none-eabi-gcc.exe: severity: message
        regexPattern: /^(?:(.*arm-none-eabi-gcc\.exe):)\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/,
        matchTypes: [MatchType.Full, MatchType.File, MatchType.Severity, MatchType.Message], 
    },
];

class Parser extends util_1.RawDiagnosticParser {
    doHandleLine(line) {
        let mat = /(.*): (In instantiation of.+)/.exec(line);
        if (mat) {
            const [, , message] = mat;
            this._pendingTemplateError = {
                rootInstantiation: message,
                requiredFrom: []
            };
            return util_1.FeedLineResult.Ok;
        }
        if (this._pendingTemplateError) {
            mat = /(.*):(\d+):(\d+):(  +required from.+)/.exec(line);
            if (mat) {
                const [, file, linestr, column, message] = mat;
                const lineNo = (0, util_1.oneLess)(linestr);
                this._pendingTemplateError.requiredFrom.push({
                    file,
                    location: new vscode.Range(lineNo, parseInt(column), lineNo, 999),
                    message
                });
                return util_1.FeedLineResult.Ok;
            } else {
                mat = /(.*):(\d+):(  +required from.+)/.exec(line);
                if (mat) {
                    const [, file, linestr, message] = mat;
                    const lineNo = (0, util_1.oneLess)(linestr);
                    this._pendingTemplateError.requiredFrom.push({
                        file,
                        location: new vscode.Range(lineNo, parseInt("1"), lineNo, 999),
                        message
                    });
                    return util_1.FeedLineResult.Ok;
                }
            }
        }
        // Early-catch backtrace limit notes
        mat = /note: \((.*backtrace-limit.*)\)/.exec(line);
        if (mat && this._prevDiag && this._prevDiag.related.length !== 0) {
            const prevRelated = this._prevDiag.related[0];
            this._prevDiag.related.push({
                file: prevRelated.file,
                location: prevRelated.location,
                message: mat[1]
            });
            return util_1.FeedLineResult.Ok;
        }
        // Test if this is a diagnostic
        mat = null;
        
        let full = null;
        let file = null; 
        let lineno = (0, util_1.oneLess)("1");
        let columnno = (0, util_1.oneLess)("1");
        let severity = 'error';
        let message = null; 
        
        for(const [index, regexPattern] of regexPatterns.entries()) {
            mat = line.match(regexPattern.regexPattern);
            
            if (mat !== null) {   
                for (let i = 0; i < mat.length; i++) {
                    switch(regexPattern.matchTypes[i]) {
                    case MatchType.Full:
                        full = mat[i];
                        break;
                    case MatchType.File:
                        file = mat[i];
                        break;
                    case MatchType.Line:
                        lineno = (0, util_1.oneLess)(mat[i]);
                        break;
                    case MatchType.Column:
                        columnno = (0, util_1.oneLess)(mat[i]);
                        break;
                    case MatchType.Severity:
                        severity = mat[i];
                        break;
                    case MatchType.Message:
                        message = mat[i];
                        break;
                    default:
                        break;
                    }
                }
                break;
            }
        }

        if (!mat) {
            // Nothing to see on this line of output...
            return util_1.FeedLineResult.NotMine;
        }
        else {
            if (severity === 'note' && this._prevDiag) {
                this._prevDiag.related.push({
                    file,
                    location: new vscode.Range(lineno, columnno, lineno, 999),
                    message
                });
                return util_1.FeedLineResult.Ok;
            }
            else {
                const related = [];
                const location = new vscode.Range(lineno, columnno, lineno, 999);
                if (this._pendingTemplateError) {
                    related.push({
                        location,
                        file,
                        message: this._pendingTemplateError.rootInstantiation
                    });
                    related.push(...this._pendingTemplateError.requiredFrom);
                    this._pendingTemplateError = undefined;
                }
                return this._prevDiag = {
                    full,
                    file,
                    location,
                    severity,
                    message,
                    related
                };
            }
            return util_1.FeedLineResult.NotMine;
        }
    }
}

To use this parser instead of the original one, you have to modify the main.js in your vscode-cmake-tools, which is located here: C:\Users\your_username\.vscode\extensions\ms-vscode.cmake-tools-1.18.42\dist\main.js

The parser code starts at line L52429 and ends at line L52516.

Just replace the code form start line to end line, perform Reload Window if vscode is opened, and the modified version should be running without further ado.

Output looks like this now: image

The first undefined reference error is a linker error without a column and without a severity. I is not parsed without the modifications. The ld.exe errors are special linker errors without a proper gcc warninig format. using 'null' or '/' as suggested by @RolfNoot did not generate a proper non-file-URI, so I left it at the ld.exe. The empty_test warning is a gcc compiler warning without a column. I is not parsed without the modifications.

This has not been thoroughly tested. It is a work in progress. It will probalby need some refinements, it might even detect some false positives, or not work for Linux developers etc.

I probably did not handle the if (this._pendingTemplateError) part correctly since I did not use all new regex cases. I don't care for now because it appears to be an error case.

Hope this helps somebody.

Cheers

Edit 1: As expected, there are some more linker errors with different format. I modified the typescript playground and the parser code to handle the linker error path/to/file1:40: multiple definition of 'symbol'; relative/path/to/objfile2.c.obj:path/to/file2:77: first defined here : grafik

Since I expect more additions for linker warnings with a weird format resulting in the need for more regex patterns, I made the code a little more generic so that only the pattern and the match types need to be added and no logic must be touched anymore.

Edit 2: Added a further linker error detection (syntax error). Fixed linker error detection regexes.

Edit 3: Added further linker error dections, updated instructions to new extension version 1.18.41.

0xemgy avatar Mar 16 '24 16:03 0xemgy

@0xemgy Thanks for the investigation and comments.

This seems like a GREAT opportunity for you to improve the extension for everyone by making an Open Source Contribution! Is this something you feel confident would work for everyone and would be willing to PR?

gcampbell-msft avatar Mar 26 '24 13:03 gcampbell-msft

@0xemgy Thanks for the investigation and comments.

This seems like a GREAT opportunity for you to improve the extension for everyone by making an Open Source Contribution! Is this something you feel confident would work for everyone and would be willing to PR?

I agree that this seems like a great opportunity and I would be willing to contribute.

But right now I don't feel confident that this would work for everyone. Because:

  1. I basically ignored the if (this._pendingTemplateError) part, where the original regex pattern is used too and might be enhanced to use all of the new regex patterns instead. I don't even know what that error is and how to test this.
  2. I did not test on Linux, I'm a windows only user and have little to no experience with Linux.
  3. After just 1 week of using my first version at work I encountered 2-3 further linker warnings/errors, one of them being accidentally parsed by one of the new regex patterns in an incorrect way.
  4. There are probably a few more linker warnings in weird formats.

Definitely no insurmountable problem there, but quite a few aspects to consider.

So I'd say I will test this further, edit my original comment if I have added/fixed something, and do some research for Linux, the template error case, and further common linker errors.

And then I will attempt to contribute, which would be my first contribution ever, so I would need to see how that works too.

Depending on my spare time this might take a few weeks or months.

Does that sound like a plan or do you have a better suggestion?

0xemgy avatar Mar 27 '24 09:03 0xemgy

@0xemgy Thank you for all of that context.

That sounds like a great plan! If we end up deciding to prioritize this issue such that we also start investigating and working on it, we will make sure to follow up here so that we can sync and make sure that no work is duplicated. Thank you.

gcampbell-msft avatar Mar 27 '24 17:03 gcampbell-msft

@0xemgy Thank you so much for your response and your contribution to the community! If you make any progress, we'll follow up on this here as well after you update the comment or add a new one.

v-frankwang avatar Jul 03 '24 09:07 v-frankwang

@gcampbell-msft @v-frankwang I was able to test my solution throughout the last months (using it almost every day at work).

Regarding my previous concerns:

  1. I left the pending error code unchanged. Hence nothing should have been broken.
  2. I did not test on Linux. I'm a pure Windows user, but I wouldn't know why it should not work on Linux.
  3. I tested it extensively as mentioned above (I did not find any false positives, and all the warnings I was able to generated seemed to be parsed successfully).
  4. I did find a few more warnings and added them.

So now I'm confident in my solution and I am willing to PR.

I already created a branch locally, modified the gcc.ts and was able to build it, test the parsing with my local software project, and ran the unit tests without an error.

But I am not allowed to push: You don't have permission to push to "microsoft/vscode-cmake-tools" on GitHub.

0xemgy avatar Jul 27 '24 10:07 0xemgy

@gcampbell-msft The user has created a branch of his own with his changes, but cannot push to the corresponding branch of microsoft/vscode-cmake-tools, can you give the user some advice?

v-frankwang avatar Jul 29 '24 02:07 v-frankwang

@v-frankwang @0xemgy Thanks for all of your work and your updates! Let's get it such that you can contribute.

The reason that you are getting the permission errors is because in order to create a PR, you should first create a fork of the cmake-tools repository, and then you can create a PR from that into this repository. Does that make sense? Please let me know if you need/want any help with this.

gcampbell-msft avatar Jul 30 '24 15:07 gcampbell-msft

@gcampbell-msft @v-frankwang I think I got it: Forked the repo, created a PR, agreed to the CLA: https://github.com/microsoft/vscode-cmake-tools/pull/3950

0xemgy avatar Aug 01 '24 05:08 0xemgy

@gcampbell-msft @v-frankwang

It turns out there already is a GNU linker error parser in your code base.

The CI unit test in my PR #3950 failed because the unit test for this linker error parser failed. It now fails because my GCC error parser already finds linker errors, which resulsts in the linker error parser not being executed,

It turns out you guys have a bug that causes errors that are detected by the linker error parser to not be pushed to the Problems View (short description of the bug: default configuration string for linker parser is "gnuld", but code uses "link", so linker errors are ignored in the function resolveDiagnostics) . If fixed this bug locally by changing the string "link" to "gnuld".

Then errors detected by the linker error parser are shown in the Problems View. But the parsing does not work properly. It is too simplistic. It only detects 6 out of the 12 linker errors I use for testing, and 3 out of these 6 findings are parsed incorrectly (e.g. path/to/ld.exe:path/to/file:138: syntax error is detected, but the file name path/to/ld.exe:path/to/file is parsed instead of path/to/file).

So I suggest I fix the described bug and move the linker errors that I implemented in the GCC parser to the linker parser, and also add some unit tests for all the error strings I use for testing.

I also would like to test this new solution at work for a few weeks to make sure I did not break anything. I think I need to test it for a while because the parsing order is actually relevant. E.g. a linker error of type path/to/ld[.exe]:path/to/file:line: warning: memory region ... not declared would be falsely detected as a GCC compiler error of type path/to/file:line: severity: message` if the compiler errors are parsed prior to the linker errors. I already regarded this in my local fix by changing the parser execution order (linker parser is now executed before the compiler parser). But I prefer to test this a little further.

Are you guys fine with that or do you prefer my current solution with linker error parsing in the GCC parser and want to delete the separate linker error parser?

0xemgy avatar Aug 03 '24 13:08 0xemgy

I like to test as well, working a lot with gcc. Is there a repo or .vsix to test it?

RolfNoot avatar Aug 03 '24 17:08 RolfNoot

@RolfNoot I created a new branch in my forked repo, see branch dev/0xemgy/FixGnuLdProblemsViewPrompt here, with the solution I'm confident @gcampbell-msft will prefer (keeping the gnu ld parser).

I generated a vsix (with local changes to the version in package.json to 99.99.0): cmake-tools-99.99.0.vsix.zip Edit: 99.99.1 cmake-tools-99.99.1.zip

I tested it rudimentarily:

  1. Installed the extension manually, overwriting the latest release: grafik

  2. Made sure my code is really in the main.js of the extension now running (see "C:\Users\my_username.vscode\extensions\ms-vscode.cmake-tools-99.99.0\dist\main.js": grafik

  3. Tested it with just one warning (will test it extensively in the following weeks): grafik Undefined reference is detected successfully.

I also updated the typescript playground code where I tested the regex stuff over the last couple of months (see here).

So at first glance looking good, but since I am not proficient at all with typescript and desktop applications in general, I might have overlooked something. Looking forward to your feedback!

0xemgy avatar Aug 04 '24 14:08 0xemgy

@0xemgy Thanks for all of your work! Yes, I agree that we should solve that bug and am happy for you to implement and continue testing,

gcampbell-msft avatar Aug 05 '24 09:08 gcampbell-msft