Skip to content

CPP: Revert recent microsoft detection change#1216

Merged
semmle-qlci merged 2 commits into
github:masterfrom
geoffw0:revert-microsoft
Apr 6, 2019
Merged

CPP: Revert recent microsoft detection change#1216
semmle-qlci merged 2 commits into
github:masterfrom
geoffw0:revert-microsoft

Conversation

@geoffw0
Copy link
Copy Markdown
Contributor

@geoffw0 geoffw0 commented Apr 5, 2019

Reverts #1191.

This will re-introduce the 239 WrongTypeFormatArgument FPs on ChakraCore on the differences job - but fix the Windows tests.

Annoyingly I can't actually find a snapshot where the original problem (failing to detect that files in a ChakraCore snapshot are built as Microsoft) actually occurs. With such a snapshot I could analyse exactly the clues that are present and possibly develop a better solution.

@geoffw0 geoffw0 added the C++ label Apr 5, 2019
@geoffw0 geoffw0 requested a review from a team as a code owner April 5, 2019 16:41
@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented Apr 5, 2019

I've figured out that, in the ChakraCore snapshot in question, some compilations do and some do not include a reference to cl.exe (hence the incorrect results for formatting functions). Rather than try to make compiledAsMicrosoft more accurate, it should suffice to check whether any file was detected as compiledAsMicrosoft. This assumes no mixed-compiler builds, but other than that I think it should be accurate.

Copy link
Copy Markdown
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious what the command lines without cl.exe look like. Anyway, let's merge this as a first step, and then we can take a bit longer to understand the problem and test the proper fix.

@semmle-qlci semmle-qlci merged commit 0bd4fde into github:master Apr 6, 2019
@geoffw0
Copy link
Copy Markdown
Contributor Author

geoffw0 commented Apr 8, 2019

They look something like this (comma-separated arguments):

<compilation #1>	F:\Jenkins\workspace\Build-Snapshot\chakracore_windows@2\odasa\odasa\tools/extractor.exe, --mimic, @-

I'm working on a PR that will fix this by only caring whether any file in the snapshot contained cl.exe (or similar). This should be reliable in all cases except where the snapshot contains a mix of compilers (which I've never actually seen outside of our tests).

@jbj
Copy link
Copy Markdown
Contributor

jbj commented Apr 8, 2019

That sounds reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants