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

WIP: Introduce a compile-time flag to diagnose file locking issues #1368

Open
wants to merge 79 commits into
base: main
Choose a base branch
from

Conversation

dscho
Copy link
Member

@dscho dscho commented Nov 18, 2017

There seem to be some problems with Git, or with GVFS Git, where git.exe itself would have open file handles to files it wants to overwrite. That does not work.

To make this notoriously-hard-to-debug problem easier to diagnose, let's add a mode where Git for Windows records stacktraces for every file it opened (forgetting about those it already closed again), and then show those stacktraces when we detect that we cannot overwrite or delete a given file.

This is a Work-In-Progress, both to serve as a back-up, as well as to be open to interested lurkers who may want to take a stab at it (as I won't be able to work on this for at least a week).

This is a standalone version of the libbacktrace library that I
originally wrote for GCC.  This is a copy of libbacktrace from GCC
trunk, with all dependencies incorporated here.
@dscho dscho force-pushed the stacktrace branch 3 times, most recently from 4a0cc10 to 6866f7f Compare November 19, 2017 21:05
@larsxschneider
Copy link
Member

Great idea! 👍 That should help us with #1262 and others. Did you consider a run-time flag (e.g. GIT_TRACE_FILE_LOCKS=1)? These problems usually happen on some developers machine with a certain repo and a certain configuration only. With a compile-time flag I would need to replicate the conditions on my machine to debug it.

I know you how much you dislike submodules... this is the situation were I usually use them to keep the PR small 😄

@dscho
Copy link
Member Author

dscho commented Dec 9, 2017

Did you consider a run-time flag

Yes. It's not working, due to Address Space Layout Randomization: that causes the stacktraces not to include symbols. Rather pointless, then.

I know you how much you dislike submodules

No, I hate them. And with lots of reasons. Just try to rebase a patch series where a submodule was introduced in the middle. Just try it.

thanm and others added 17 commits January 12, 2018 10:26
Includes (among other things) support for compressed debug sections,
a variety of bugfixes, and expanded test coverage.
Previous change (update from gcc trunk) broke 'make install'; this
corrects the problem.
This brings in this patch:

	* elf.c (codes) [GENERATE_FIXED_HUFFMAN_TABLE]: Fix size to be
	288.
	(main) [GENERATE_FIXED_HUFFMAN_TABLE]: Pass 288 to
	elf_zlib_inflate_table.  Generate elf_zlib_default_dist_table.
	(elf_zlib_default_table): Update.
	(elf_zlib_default_dist_table): New static array.
	(elf_zlib_inflate): Use elf_zlib_default_dist_table for dist table
	for block type 1.
	* ztest.c (struct zlib_test): Add uncompressed_len.
	(tests): Initialize uncompressed_len field.  Add new test case.
	(test_samples): Use uncompressed_len field.
Add required support files to the config subdirectory.

Update dtest to be based on a statically linked program.
	desired CRC is zero.
	(elf_add): Don't clear *found_sym and *found_dwarf if debuginfo.
	* btest.c (check_open_files): New static function.
	(main): Call check_open_files.
2018-04-17  Ian Lance Taylor  <[email protected]>

	* backtrace.c (backtrace_full): When testing whether we can
	allocate memory, call mmap directly, and munmap the memory.

2018-04-04  Jakub Jelinek  <[email protected]>

	PR other/85161
	* elf.c (elf_zlib_fetch): Fix up predefined macro names in test for
	big endian, only use 32-bit loads if endianity macros are predefined
	and indicate big or little endian.

2018-02-15  Jakub Jelinek  <[email protected]>

	PR other/82368
	* elf.c (SHT_PROGBITS): Undefine and define.

2018-02-14  Jakub Jelinek  <[email protected]>

	PR other/82368
	* elf.c (EM_PPC64, EF_PPC64_ABI): Undefine and define.
	(struct elf_ppc64_opd_data): New type.
	(elf_initialize_syminfo): Add opd argument, handle symbols
	pointing into the PowerPC64 ELFv1 .opd section.
	(elf_add): Read .opd section on PowerPC64 ELFv1, pass pointer
	to structure with .opd data to elf_initialize_syminfo.

2018-01-19  Tony Reix  <[email protected]>

	* xcoff.c (xcoff_incl_compare): New function.
	(xcoff_incl_search): New function.
	(xcoff_process_linenos): Use bsearch to find include file.
	(xcoff_initialize_fileline): Sort include file information.

Fixes #13
Update to config.guess 2018-05-19 and config.sub 2018-05-24 from
git://git.savannah.gnu.org/config.git revision
3a2a927f547ee478147008c3fda2adb8a2b2ebc4.

This brings in musl support.
@lelanvy18

This comment has been minimized.

This adds DWARF 5 support as well as an enhanced testsuite.

Patch assembled by Than McIntosh.
This supports FreeBSD and NetBSD when /proc is not mounted.
ianlancetaylor and others added 29 commits October 20, 2020 11:50
Use an attribute rather than a comment when falling through a switch case.

	* internal.h (ATTRIBUTE_FALLTHROUGH): Define.
	* elf.c (elf_zlib_inflate): Use ATTRIBUTE_FALLTHROUGH.
	* dwarf.c (resolve_string): Use > rather than >= to check whether
	string index extends past buffer.
	(resolve_addr_index): Similarly for address index.
	PR debug/98716
	* dwarf.c (read_v2_paths): Allocate zero entry for dirs and
	filenames.
	(read_line_program): Remove parameter u, change caller.  Don't
	subtract one from dirs and filenames index.
	(read_function_entry): Don't subtract one from filenames index.
	* Makefile.am (%_dwz): If dwz fails, use uncompressed debug info.
	* Makefile.in: Regenerate.
	* configure.ac: Check for objcopy --add-gnu-debuglink by using
	objcopy --help.
	* configure: Regenerate
	PR libbacktrace/98818
	* dwarf.c (dwarf_buf_error): Add errnum parameter.  Change all
	callers.
	* backtrace.h: Update backtrace_error_callback comment.
It's no longer necessary as file 0 is now set up in all cases.

	* dwarf.c (read_line_program): Don't special case file 0.
	(read_function_entry): Likewise.

Fixes msysgit#69
When I added the fields in 2019-12-13 I forgot to initialize them.

	* dwarf.c (build_address_map): Initialize DWARF 5 fields of unit.
	* dwarf.c (find_address_ranges): Handle skeleton units.
	(read_function_entry): Likewise.
Patch from Rui Ueyama, who says:

libbacktrace occasionally fails to decompress compressed debug info
even though the sections contain valid zlib streams. The cause of the
issue is an off-by-one error.

If a zlib data block is a plain data (uncompressed data), the next two
bytes contain the size of the block. These two bytes value is byte-
aligned, so if we read-ahead more than 8 bits, we need to unread it.

So, the correct condition to determine whether or not we need to
unread a byte is bits >= 8 and not bits > 8. Due to this error,
if the last read bits happened to end at a byte boundary, the next
byte would be skipped. That caused the decompression failure.

This bug was originally reported against the mold linker.
rui314/mold#402

	* elf.c (elf_zlib_inflate): Don't skip initial aligned byte in
	uncompressed block.
Patch by Xi Ruoyao.

	* configure.ac: Use grep instead of fgrep.
	* configure: Regenerate.
	* Makefile.am (MAKETESTS): New variable split out of TESTS.
	(CLEANFILES): Replace TESTS with BUILDTESTS and MAKETESTS.
	* Makefile.in: Regenerate.

Fixes msysgit#81
	* macho.c (backtrace_initialize) [HAVE_MACH_O_DYLD_H]: Don't exit
	loop if we can't find debug info for one shared library.

For msysgit#85
QNX uses sys/link.h rather than link.h for dl_iterate_phdr

Fixes msysgit#86

	* configure.ac: Check for sys/link.h.  Use either link.h or
	sys/link.h when checking for dl_iterate_phdr.
	* elf.c: Include sys/link.h if available.
	* configure, config.h.in: Regenerate.
In rust-lang/rust#39468 it was discovered that this could cause a crash in
libbacktrace due to freeing uninitialized memory, and this specific instance was
fixed in rust-lang/rust#39509
This updates the local declaration of `str_size` to always be 4 bytes instead of
platform-dependent as its initialization later on only fills in 4 bytes instead
of all the bytes of `size_t`.

Originally reported as rust-lang/rust#28447 this was fixed in
rust-lang/rust#30908
Note: as we target MINGW here, we still want to look up the symbols via
the DWARF method (the native Windows way would be to call the
SymFromAddr() function, but that would require .pdb files which MINGW
does not produce).

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Merge the `windows` branch of https://github.com/dscho/libbacktrace
(itself a fork of https://github.com/ianlancetaylor/libbacktrace) into
compat/libbacktrace/, for use primarily by compat/mingw.c to generate
(you guessed it) stack traces.

This trick was performed by:

	git fetch https://github.com/dscho/libbacktrace windows
	git read-tree --prefix=compat/libbacktrace/ -u FETCH_HEAD
	git reset $(git commit-tree -m "Add libbacktrace" \
		-p HEAD -p FETCH_HEAD $(git write-tree))

and then amending the commit to edit the commit message ;-)

Signed-off-by: Johannes Schindelin <[email protected]>
Git's DEVELOPER CFLAGS do not allow it.

Signed-off-by: Johannes Schindelin <[email protected]>
This is not really the proper way to include them, but it will have to
do for now.

Signed-off-by: Johannes Schindelin <[email protected]>
On Windows, it is not possible to overwrite files as long as they are in
use. This is particularly funny (not!) when the same process still holds
open file handles to that file, and it notoriously hard to debug.

Let's add a compile-time option to output stacktraces to the offending
calls when we fail to overwrite/remove them.

Note: it has to be a compile time option because we would have to turn
off ASLR otherwise (and it is too good of a first line of defense to
just turn off).

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants