Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC linker errors not showing up in Problems window #2864

Open
RolfNoot opened this issue Nov 22, 2022 · 28 comments
Open

GCC linker errors not showing up in Problems window #2864

RolfNoot opened this issue Nov 22, 2022 · 28 comments
Labels
bug a bug in the product Feature: diagnostics issues with problem matchers for compiler output help wanted we currently are not planning work on this and would like help from the open source community
Milestone

Comments

@RolfNoot
Copy link

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

@bobbrow
Copy link
Member

bobbrow commented Nov 22, 2022

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 bobbrow added bug a bug in the product help wanted we currently are not planning work on this and would like help from the open source community Feature: diagnostics issues with problem matchers for compiler output labels Nov 22, 2022
@bobbrow bobbrow added this to the Backlog milestone Nov 22, 2022
@RolfNoot
Copy link
Author

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

@bobbrow
Copy link
Member

bobbrow commented Nov 22, 2022

It is here:

export const REGEX = /^(.*):(\d+):(\d+):\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/;

@RolfNoot
Copy link
Author

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')?

@bobbrow
Copy link
Member

bobbrow commented Nov 23, 2022

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.

@evakili
Copy link

evakili commented Jan 4, 2023

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

@denis-shienkov
Copy link

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.

@RolfNoot
Copy link
Author

RolfNoot commented Feb 3, 2023

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
            }
        }

@denis-shienkov
Copy link

denis-shienkov commented Feb 4, 2023

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:

but all was unsucessfull.

@RolfNoot
Copy link
Author

RolfNoot commented Feb 4, 2023

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.

@denis-shienkov
Copy link

denis-shienkov commented Feb 4, 2023

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).

@RolfNoot
Copy link
Author

RolfNoot commented Feb 4, 2023

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.

@kuch3n
Copy link

kuch3n commented Feb 17, 2024

It is here:

export const REGEX = /^(.*):(\d+):(\d+):\s+(?:fatal )?(\w*)(?:\sfatale)?\s?:\s+(.*)/;

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

@0xemgy
Copy link

0xemgy commented Mar 16, 2024

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.

@gcampbell-msft
Copy link
Collaborator

@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?

@0xemgy
Copy link

0xemgy commented Mar 27, 2024

@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?

@gcampbell-msft
Copy link
Collaborator

gcampbell-msft commented Mar 27, 2024

@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.

@v-frankwang
Copy link
Collaborator

@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.

@0xemgy
Copy link

0xemgy commented Jul 27, 2024

@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.

@v-frankwang
Copy link
Collaborator

@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?

@gcampbell-msft
Copy link
Collaborator

@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 gcampbell-msft modified the milestones: Backlog, On Deck Jul 30, 2024
@gcampbell-msft gcampbell-msft assigned 0xemgy and unassigned 0xemgy Jul 30, 2024
@0xemgy
Copy link

0xemgy commented Aug 1, 2024

@gcampbell-msft @v-frankwang I think I got it: Forked the repo, created a PR, agreed to the CLA: #3950

@0xemgy
Copy link

0xemgy commented Aug 3, 2024

@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?

@RolfNoot
Copy link
Author

RolfNoot commented Aug 3, 2024

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

@0xemgy
Copy link

0xemgy commented Aug 4, 2024

@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!

@gcampbell-msft
Copy link
Collaborator

@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,

@0xemgy
Copy link

0xemgy commented Aug 25, 2024

@gcampbell-msft I was able to test my changes extensively over the last 3 weeks at work. I also added a few unit tests and cleaned up the regex expressions. For my use cases it is working properly. It is commited in the PR 3950 and is ready for review.

Of course, feedback of @RolfNoot would be much appreciated.

@v-frankwang
Copy link
Collaborator

@gcampbell-msft The user made some changes to the PR: #3950 based on the survey and asked you to review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the product Feature: diagnostics issues with problem matchers for compiler output help wanted we currently are not planning work on this and would like help from the open source community
Projects
Status: Pending Prioritization
Development

No branches or pull requests

8 participants