forked from ccache/ccache
-
Notifications
You must be signed in to change notification settings - Fork 0
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
PR: Code QL #4
Open
AlexanderLanin
wants to merge
38
commits into
master
Choose a base branch
from
codeql
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
PR: Code QL #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexanderLanin
force-pushed
the
master
branch
from
November 27, 2020 07:09
1cb77f6
to
37ab1b3
Compare
AlexanderLanin
force-pushed
the
codeql
branch
from
December 11, 2020 08:40
3bfe84f
to
a90df40
Compare
AlexanderLanin
force-pushed
the
codeql
branch
2 times, most recently
from
December 11, 2020 09:45
c83877f
to
3657d8c
Compare
Unless when compiling a preprocessed file directly, ccache creates a temporary file to store the output of the preprocessor, registers the file for removal at program exit, renames the file to one with a .i/.ii extension and then registers that file for removal as well. This works by chance in practice as long as mkstemp() returns something with low probability of being reused, but as discussed in ccache#736 it risks failing when mkstemp() doesn’t behave that way. Fix this by creating the new name (with the needed extension) using a hard link so that the original file will outlive the new file, thus blocking another ccache process from creating a file with the same name again. To make the new temporary file outlive the old file, also delete pending temporary files in LIFO order.
GitHub Discussions is preferred now.
On Windows, multiple ccache process could race each other to create, rename and delete temporary files, because they would attempt to generate the same sequence of temporary file names (`tmp.cpp_stdout.iG2Kb7`, `tmp.cpp_stdout.P1kAlM`, `tmp.cpp_stdout.FzP5tM`, ...). This is because ccache used mingw-w64's [implementation of mkstemp][1], which uses `rand()` to generate temporary file names, and ccache was never seeding the thread-local PRNG used by `rand()`. Replace ccache's use of `mkstemp()` on Windows with an implementation based on OpenBSD. This allows us to sidestep mingw-w64's problematic implementation, and allows us to build using MSVC again. (MSVC's C standard library does not provide `mkstemp()`.) Example errors: - Some ccache process is in the process of deleting a temporary file: ccache: error: Failed to create temporary file for C:\Users\someuser/.ccache/tmp/tmp.cpp_stdout.FzP5tM: Access is denied. - Some ccache process has destination file open, so it can't be overwritten: ccache: error: failed to rename C:\Users\someuser/.ccache/tmp/tmp.cpp_stdout.iG2Kb7 to C:\Users\someuser/.ccache/tmp/tmp.cpp_stdout.iG2Kb7.ii: Access is denied. - Source file has been deleted by some other ccache process: ccache: error: failed to rename C:\Users\someuser/.ccache/tmp/tmp.cpp_stdout.P1kAlM to C:\Users\someuser/.ccache/tmp/tmp.cpp_stdout.P1kAlM.ii: The system cannot find the file specified. [1]: https://github.com/mirror/mingw-w64/blob/v8.0.0/mingw-w64-crt/misc/mkstemp.c
Bash tests were not actually being run on the macOS CI agents because the version of sed installed there does not understand the `-r` flag: sed: illegal option -- r usage: sed script [-Ealn] [-i extension] [file ...] sed [-Ealn] [-i extension] [-e script] ... [-f script_file] ... [file ...] - use `sed -E` instead of `sed -r` as the latter isn't supported by BSD sed. - export `SDKROOT` in `test/run`. Otherwise it appears at least some some Apple toolchains (e.g. Xcode 10.3) will pick the _latest_ SDK installed on the host (e.g. `/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk`) instead of using the SDK bundled with the toolchain (e.g. `/Applications/Xcode_10.3.app/.../MacOSX10.14.sdk`). The 10.15 SDK is not compatible with Xcode 10.3: ld: unsupported tapi file type '!tapi-tbd' in YAML file '/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/lib/libSystem.tbd' for architecture x86_64 clang: error: linker command failed with exit code 1
Util::dir_name does not understand “C:\”-style Windows path so add such knowledge.
The fallback replacement of realpath(3) (from 8e918cc) uses readlink(3) under the assumption that we’re only interested about symlinks, but that’s no longer the case: we’re using it for normalization purposes as well. Let’s just remove it. If it turns out that there still are non-Windows systems that don’t have realpath(3) and that we care about we’ll figure out something else.
If a non-Clang compiler gets -f(no-)color-diagnostics then bail out and just execute the compiler. The reason is that we don't include -f(no-)color-diagnostics in the hash so there can be a false cache hit in the following scenario: 1. ccache gcc -c example.c # adds a cache entry 2. ccache gcc -c example.c -fcolor-diagnostics # unexpectedly succeeds Closes: ccache#740.
AlexanderLanin
force-pushed
the
codeql
branch
2 times, most recently
from
December 30, 2020 22:42
134f2df
to
c76d525
Compare
(cherry picked from commit 5bcba58358ef2e88e1cc910583830c6830f6c712)
ResultRetriever::on_entry_data assumes that a destination file has been opened if the entry type is not stderr_output, but that’s incorrect since on_entry_start doesn’t open a destination file if it’s /dev/null. An assertion is triggered: ccache: ResultRetriever.cpp:129: virtual void ResultRetriever::on_entry_data(const uint8_t *, size_t): failed assertion: (m_dest_file_type == FileType::stderr_output && !m_dest_fd) || (m_dest_file_type != FileType::stderr_output && m_dest_fd) Fix this by letting on_entry_data handle the “destination file not opened” case and correcting the assert. Fixes ccache#752.
This avoids an “implicit capture of 'this' via '[=]' is deprecated” warning when building for C++20 and does no harm for earlier versions.
This makes the code base use nonstd::string regardless of the C++ target version, which avoids some compatibilty issues. This decision can be revisited in the future when C++17 is the lower bar. Closes ccache#749.
As mentioned in ccache#750.
…ccache#760) Scenario: - /tmp is a symlink to /private/tmp. - Both apparent and actual CWD is /private/tmp/dir. - A path (e.g. the object file) on the command line is /tmp/dir/file.o. - Base directory is set up to match /tmp/dir/file.o. Ccache then rewrites /tmp/dir/file.o into ../../private/tmp/dir/file.o, which is correct but not optimal since ./file.o would be a better relative path. Especially on macOS where, for unknown reasons, the kernel sometimes disallows opening a file like ../../private/tmp/dir/file.o for writing. Improve this by letting Util::make_relative_path try to match real_path(path) against the CWDs and choose the best match. Closes ccache#724.
This makes it possible to store debug files outside a transient build directory. As a bonus, it also allows for getting debug files when the object file is /dev/null.
Add VS2019 build jobs that use clang for the test suite. There are many test failures on Windows, but these are ignored for now. Tweak CMake build scripts: - Fix CI build type handling for MSVC (recognise `/NDEBUG` and not just `-DNDEBUG`) - Fix incorrect warnings-as-errors flag for MSVC - Suppress an additional conversion warning on MSVC
Version 2.4.4 includes the fix in PR ccache#743 for issue ccache#731.
sigaddset and similar functions are specified by POSIX to be in signal.h and the C++ csignal header only contains a subset of the signal.h declarations. Related to PR ccache#758.
By adding . as an include directory, CMake actually took it literally and files are included from e.g. src/./AtomicFile.h. However in .clang-tidy headers with an additional slash (headers from subdirectory third_party) are excluded from reports. This commit: - gets rid of include via . - fixes some warnings - disables the rest
SOURCE_DATE_EPOCH will be passed from debhelpers, by extracting last entry from d/changelog (or current time if there is entries) And this will not allow to use cache.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.